Jump to content

  • Log In with Google      Sign In   
  • Create Account

Banner advertising on our site currently available from just $5!


1. Learn about the promo. 2. Sign up for GDNet+. 3. Set up your advert!


rip-off

Member Since 16 Mar 2005
Offline Last Active Today, 12:30 PM

#5207845 Low Level Memory Management C++

Posted by rip-off on Today, 06:44 AM

Your smart_array assignment operators appear to be incorrect, it doesn't decrement the reference count of the current object (if any). Also, I think it is surprising and not advisable to support arbitrary pointer identity comparison. I'd recommend instead having an explicit member function if this is actually required. The detach member function seems poorly designed, it doesn't guarantee ownership transfer (as other references may still exist), so the client cannot safely assume ownership of the returned pointer.

As for the ArrayList, the class is certainly not thread safe. Generally, it is really hard to provide thread safety at such a low level, as most client code will need to make multiple calls to the object to achieve many tasks. If you wish to provide a thread safe collection, I'd recommend a separate class (possibly making using of this class, or std::vector<> internally) with a limited, "transactional" oriented interface. The limited interface makes it easier to validate the thread safety. In addition, you are using a mixture of low and high level thread primitives, I'd recommend picking one (e.g. mutex locking) and using it consistently (e.g. the entry point to every public member function).

Other than that, I find many of the member functions to be very surprising. For example, the difference between size() and count() is very subtle and I would say unnecessarily error prone. The behaviour of resize() is bizarre, it does nothing if the array is smaller than specified, otherwise it expands the array by the number of elements passed.

In any case, one issue is that the smart_array assumes it can delete[] the pointer it manages, but the wrapping ArrayList is giving it pointers that were not allocated with new[].

Overall, I find the classes a confusing mix of thread-safety, reference counting, memory management and dynamic array logic. As mentioned above, the first thing I would recommend is removing the thread safety from the equation. You can write a thread-safe wrapper for this later - if it is really required! Ideally, you try not to share objects between threads when you can avoid it, the only objects that should be shared are thread-oriented things like work queues for producer / consumer relationships. I'd also recommend dropping the reference counting, again if required this can be handled at a higher level.

Once you've dropped these, then it becomes clear that you're essentially re-implementing std::vector. Consider not doing that, but instead using the tried and tested implementation. You can then wrap something like that in a thread safe interface, if that is really something your program requires.


#5207410 Is it a very bad idea for a class to pass (*this) into a function? Strange bu...

Posted by rip-off on 29 January 2015 - 05:38 AM

The solution is to understand the Rule of Three (this rule has recently grown a bunch of different names, but I think this still is the best name).

 

So, your BitmapFont class or potentially some of the other classes it uses, should be marked as "noncopyable". You can achieve that in C++11 by "deleting" the copy constructor and assignment operator, and in pre C++11 you can simulate this by declaring them private and not implementing them.

 

This will address the issue you mentioned of the silent bug lying around. Anywhere you are copying these objects should now give a compilation error (or possibly a link error).

 

The next thing to consider is the design. As Washu says, this design seems weak.




#5207253 Is it a very bad idea for a class to pass (*this) into a function? Strange bu...

Posted by rip-off on 28 January 2015 - 02:08 PM

What is likely happening is that the BitmapFont is not copyable. That is, when you create a copy, and the copy goes out of scope, it destroys resources that the "original" is also using.

Something like this:
$ cat test.cpp
#include <iostream>
class Bad {
public:
    Bad() : pointer(new int(42)) {
        std::cout << "constructor: " << this << ", pointer: " << pointer << '\n';
    }

    ~Bad() {
        std::cout << "destructor: " << this << ", pointer: " << pointer << '\n';
        delete pointer;
    }

    int *pointer;
};

int main() {
    Bad original;
    Bad copy = original;
}
$ g++ test.cpp && ./a.out
constructor: 0x7fffeb5038a0, pointer: 0x131d010
destructor: 0x7fffeb5038b0, pointer: 0x131d010
destructor: 0x7fffeb5038a0, pointer: 0x131d010
*** Error in `./a.out': double free or corruption (fasttop): 0x000000000131d010 ***
Aborted (core dumped)
As you can see, there are two "this" pointer values, but only one "pointer" value. The destructor causes the pointer in "original" to also be invalidated.


#5206863 Designing a < operator for a GUID

Posted by rip-off on 27 January 2015 - 01:56 AM

You don't need GUIDs for this. GUIDs are useful if two machines that aren't currently connected together need to independently generate identifiers, which later might become connected later (and the IDs could collide).

In most networked games, this isn't the case. The computers are immediately connected, so the server can generate IDs and send them to the client.

When sending binary data across the network, you do have to be careful. You have to pay attention to the size of each field, and any padding. In addition, a binary integer has many representations, the most common of which are little endian and big endian. One typically selects an endianness for the protocol, and converts incoming multi-byte integers to that storage. Historically, big endian has been preferred, even though on modern desktops you are likely using little endian. Some network protocols use different techniques, e.g. variable length encoding depending on the value, or send a delta-value instead, possibly with run-length encoding (RLE) to handle blocks of zero values.

Sending text data doesn't have these worries, but you have other issues about parsing such as handling delimiters that occur in the data you want to send, e.g. if you use the space character as a delimiter but the user wants to send a username or a chat message that includes spaces. Textual data is typically less space efficient on the network and takes longer to read and write than binary data. Not necessarily an immediate concern, it again depends on the game and how often you need to send and the volume of data you are sending.


#5206725 Getting array boundary exception that is so weird

Posted by rip-off on 26 January 2015 - 11:20 AM

Probably needs a double-dereference, sorry. Iterators, when dereferenced, yield the container's value type - which is in this case a pointer. A second dereference is necessary to bind to a reference.




#5206689 Encapsulation through anonymous namespaces

Posted by rip-off on 26 January 2015 - 06:19 AM

The hard part is managing instantiation. To instantiate the object, C++ needs to know it's size. This means that in Oberon_Command's example, either the main.cpp file needs to be able to include a special header that gives the class definition, or you need to switch to dynamic allocation,e.g. the original header contains a factory function to return something like std::unique_ptr<Instance>.

 

That interface is very C like (opaque struct). Consider also the PIMPL idiom.




#5206686 Getting array boundary exception that is so weird

Posted by rip-off on 26 January 2015 - 06:06 AM

That implementation only increments the invader iterator if there is at least one bullet, and can increment it many times if there are more than one.

 

Consider breaking the loop(s) into a separate helper function(s):

bool checkBulletCollision(const Invader &invader, const list<Bullet *> &bullets) {
    for (list<Bullet*>::iterator it = bullets.begin(); it != bullets.end(); it++) {
        const Bullet &bullet = *it;
        float left = bullet.Position.x - 5;
        float top = bullet.Position.y - 13;
 
 
        if (CheckCollision( /* ... */) {
            // Set red
            Iw2DSetColour(0xff0000ff); 
            // Draw outline
            Iw2DDrawRect(CIwFVec2(left, top), CIwFVec2(invader.AlienSprite->m_W, invader.AlienSprite->m_H)); 
            return true;
        }
    }
    return false;
}
 
void updateInvadersAndBullets(vector<Invader *> &invaders, const list<Bullet *> &bullets) {
    vector<Invader*>::iterator it = invaders.begin()
    while (it != invaders.end()) {
        Invader &invader = *it;
        if (checkBulletCollision(invader, bullets)) {
            it = invaders.erase(it);
        } else {
            it++;
        }
    }
}
 
// Wherever
updateInvadersAndBullets(*g_Invaders.invaders, *ship.Bullets);

Note how creating named references for objects can help reduce the code density and increase readability.

Note also that using a "while" loop draws attention to the non-trivial increment logic.

 

Code neither compiled nor tested.




#5206666 Constructor gotcha's?

Posted by rip-off on 26 January 2015 - 02:45 AM

by design not passing parameters to a method that will cause an invalid state
as opposed to additional run time checks (when possible).
especially in the called code. i would think it would be better to perform such checks in the calling code before calling the object method. if your parameters are whacked, odds are you don't want to make the call anyway and want to somehow recover instead.
the easiest bugs to fix are the ones you prevent by design (or coding convention w/ strict coder discipline).
an ounce of prevention can prevent a pound of cure. likewise, an ounce of design can sometimes prevent a pound of runtime checks. <g>.

A debug-only assertion that the parameters are valid helps catch bugs (inconsistent calling code checks) and can remind you of these design choices later when you go to call a member function from a new location.


#5206615 Getting array boundary exception that is so weird

Posted by rip-off on 25 January 2015 - 03:30 PM

You have to choose between a static or dynamic container here. You're indexing like you have a 2-D grid of objects, but you're erasing elements like you have a dynamic array.

Either switch to an actual 2-D grid (e.g. array with nullable elements), or iterate over the vector's current elements, not the grid of elements you originally created.


#5206609 Is it a very bad idea for a class to pass (*this) into a function? Strange bu...

Posted by rip-off on 25 January 2015 - 02:51 PM

What is the signature of functionB()? If functionB does takes ClassA by value, is the class safely copyable, i.e. obeys the rule of three?


#5205304 Generic events - Enforcing the correct function signatures.

Posted by rip-off on 19 January 2015 - 09:47 AM

I believe it is a legacy "feature". In the earliest versions of C, there was no checking of function parameters. Functions were written like so:

someFunction(count, string)
int count;
char *string;
{
    // ...
}

Some light Googling suggests that support for this is dropped in C99. Note even the return type was omitted, in such cases "int" was assumed.

 

I believe "void" was added to the language to be explicit about functions that took no parameters and / or did not return anything.

 

Another reason to prefer the idiomatic solution!




#5205143 Generic events - Enforcing the correct function signatures.

Posted by rip-off on 18 January 2015 - 04:19 PM

I don't believe C is expressive enough to capture what you are trying to do. The C APIs I have used appear to favour the idiom of a single callback function signature with a void pointer for the variable data. The callee is expected to cast the pointer to a structure of the appropriate type (often indicated by a integral "type" parameter).


#5204582 Returning a nullptr refence, how bad is my teammate?

Posted by rip-off on 15 January 2015 - 04:26 PM

Ordinary, such code would not compile, but in this case I believe this will attempt to construct a temporary std::string (as if from a null character pointer) which most implementations will assert/guard against, but either way this an express trip through undefined behaviour land, likely destination Teh Crash.

Nice of them to test the function thoroughly though, in addition to compiling on an insufficient warning level (and without warnings as errors).


#5204536 Security for beginners

Posted by rip-off on 15 January 2015 - 02:10 PM

Security has a cost, but the runtime cost is generally low compared to the cost of developing a secure product. Of course, poor security has a cost too, just look at what happened recently to Sony. The cost of security is not obvious to most people, it is hidden but potentially can be catastrophic.

A professional developer should never agree to develop an insecure product, in s similar (but less extreme) way that a doctor should do no harm and architects should not design unstable buildings. People put trust in your product, they deserve to have that respected.

Remember, a secure product doesn't just protect you from hackers, but also yourself. For example, if the quote and edit button's functinoality were accidentally swapped in the next www.gamedev.net update, having the server check whether the forum member has permission to perform the action on that post can help limit the damage (e.g. people editing each other's posts).

To develop a secure product, you have to think like an attacker. It is not about what your product can or should do, but rather what a malicious individual can make it do. Thus, to justify a particular technical approach for security, you just have to think of an example of what would be possible without that approach. For example, a hosted system must validate input data on the backend even if the frontend does validation - as frontend validation is controlled by the client and could be by-passed. If you cannot justify a particular approach, then it isn't necessarily more secure. Arbitrary rules are security theater, and a distraction from actually security threats / weaknesses.

It is good to have some attacker personas to give some structure and context to security discussions. To take your schedule planner as an example, one persona might be a jealous ex trying to stalk one of your users. Another persona could be a spammer who wants to insert fake plans into the schedule to promote themselves. Now you can run through your application's functionality, and evaluate whether there is any risk, and how you can mitigate it.


#5203554 4X Game - Making sure players 'can't have it all'

Posted by rip-off on 11 January 2015 - 04:01 PM

You sound like you're going the right way. Playtesting will help expose if there appears to be a dominant strategy. If so, you would need to tweak the design of the weapons, hulls or enemies to try create an even balance.

If you have some A.I., you could also develop a headless simulator and simulate lots of fights with various loadouts. Leave that running for some time to gather some statistics, you might be able to find such issues earlier, allowing you to focus more of your playtesting time on tasks you cannot automate like evaluating how usable and fun your game is.




PARTNERS