Following Ethan's example of bailing (calling VVV_exit()) if
binaryBlob::unPackBinary() couldn't allocate memory, I've searched
through and found every SDL_malloc(), then made sure that if it returned
NULL, the caller would bail (because you can't do much when you're out
of memory).
There should probably be an error message printed when the process is
out of memory, but unPackBinary() doesn't print an error message for
being out of memory, so this can probably be added later. (Also we don't
really have a logging system, I'd like to have something like that added
in first before adding more messages.)
Also, this doesn't account for any allocators used by STL stuff, but
we're working on removing the STL, and allocation failure just results
in an abort anyway, so there's not really a problem there.
Wow, there are a lot of these. All of these exit paths now use
VVV_exit() instead, which attempts to save unlock.vvv and settings.vvv,
and also frees all resources so Valgrind is happy. This is a good thing,
because previously unlock.vvv/settings.vvv wouldn't be written to if we
decided to bail for a given reason.
I'm going to introduce another binaryBlob object in to the mix, and I
want to be able to re-use an existing FOREACH_TRACK #define without
having to copy-paste it again. So, TRACK_NAMES now takes in a blob
parameter, which will be passed to the temporary FOREACH_TRACK #define.
The function previously conditionally freed a m_memblocks pointer if its
corresponding m_headers was valid. This makes me slightly worried about
the possibility that memory would be allocated, but the header would
still be marked as invalid.
I don't see how that could happen, but it's better to be safe than
sorry. SDL_free() does a guaranteed NULL pointer check (like most SDL
functions), so it's okay to pass NULL pointers to it.
Just to be sure, I'm also zeroing m_memblocks and m_headers after
freeing everything in the function.
The binary blob shouldn't return an index if it ends up being invalid.
That could cause a whole lot of issues if musicclass ends up parsing the
resulting struct.
With all that said, though, musicclass doesn't check the -1 sentinel
value anyway, even though it should, but that'll be fixed later.
Okay, so basically here's the include layout that this game now
consistently uses:
[The "main" header file, if any (e.g. Graphics.h for Graphics.cpp)]
[blank line]
[All system includes, such as tinyxml2/physfs/utfcpp/SDL]
[blank line]
[All project includes, such as Game.h/Entity.h/etc.]
And if applicable, another blank line, and then some special-case
include screwy stuff (take a look at editor.cpp or FileSystemUtils.cpp,
for example, they have ifdefs and defines with their includes).
I ran the game through cppcheck and it spat out a bunch of member
attributes that weren't being initialized. So I initialized them.
In the previous version of this commit, I added constructors to
GraphicsResources, otherlevelclass, labclass, warpclass, and finalclass,
but flibit says this changes the code flow enough that it's risky to
merge before 2.4, so I got rid of those constructors, too.
I don't know how no one realized that the for-loop to (poorly)
initialize m_headers was basically unnecessary, and that the memset()
should've just been used instead. Well, except it should also be
replaced with SDL_memset(), but that's besides the point.
Also, I decided to hardcode the 128 thing less, in case people want to
fork the source code and make a build where it's changed.