ZFGC

Resources => Coding => Topic started by: MG-Zero on August 19, 2009, 10:12:52 pm

Title: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:12:52 pm
Quote
//read in tileSet URL
   length = ReadCompressed(mapFile);
   temp = new char[length];
   for (int i = 0; i < length; i++)
   {
      fread(&temp, 1, 1, mapFile);
      if (temp[ i] == '\\')
         temp[ i] = '/';
   }
   //HVSTGFX::loadPNGTiles(temp, tempHMD.tileSet, 16, 16);
   delete temp;

I have this little algorithm for reading in a string from a file.  The string is prefixed with the length, so I read that into length first and it returns 79 in this case.  Now that's all fine, but when I call new on temp..the array is bigger than 79.  As you can imagine, this is causing problems when using the string as the directory for loading an image from.  Why the hell is it allocating more memory than it should?  Either that, or does anyone know of another way to read the string into a char array?  This is reading from a binary file btw
Title: Re: Memory Allocation Issue
Post by: 4Sword on August 19, 2009, 10:22:35 pm
This is probably the wrong assumption and I am quite stupid, but if you are using character arrays instead of strings, are you anticipating the null terminator? It probably make the space you need off by a character.
Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 10:26:03 pm
Your for loop  looks messed up to me, like you should replace all instances of temp with
Code: [Select]
temp[i].

Also you are using i++ where you should use ++i. It's a simple optimisation because

++i represents
i += 1;
return i;

whilst i++ represents
int r = i;
i += 1;
return r;
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:28:17 pm
No, it's not an issue with the loop.  It reads in the string fine, but there's extra space in the array that shouldn't be there.  I just checked in the debugger, and there's about 94 characters or so in the array when there should only be 79.  

4sword, nope, not that either.  It exits my loop after it gets to the 79th character and doesn't read in any null terminators.

EDIT:  Here, this is what I mean..

Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 10:30:45 pm
I think the C/C++ allocator is somewhat wasteful, depending on the compiler it may be allocating that much space because it's more efficient speed-wise to do so.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:33:15 pm
I added a screenshot from the debugger, if that makes this any easier =\
Title: Re: Memory Allocation Issue
Post by: 4Sword on August 19, 2009, 10:35:06 pm
Yeah, I was probably just being stupid. I was not sure how a function which had an argument as a character array could properly handle that character array unless it had a null terminator in it. Otherwise I thought it'd load all the junk values after the actual characters used. I was assuming that the strings you loaded in were not all 79 characters in length.
Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 10:37:59 pm
Try some 'cold, hard debugging' (as I call it)

   length = ReadCompressed(mapFile);
   temp = new char[79];
   for (int i = 0; i < 79; i++)

oh, and it should be delete[] temp, since you're deleting an array.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:42:34 pm
Yep, that's doing the exact same thing.

I just tried it with 3 as well, it adds the same number of extra characters.
Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 10:46:56 pm
hmm, have you tried forcing it to add a null terminator to the end, just to see what that does? It may be the debugger is constantly moving along the string until it hits a null and stops.

Alternatively, any reason not to use a std::string? Load a character into a tmp, append to string, repeat until done.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:48:39 pm
I need to use a char array for the string for the loadpng function I'm using.  I guess I could load it into a string, then copy it over to a char with memcpy and work from there.
Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 10:52:46 pm
Have you tried adding a null to the end yet? Also I don't think a program can calculate string length without an ending null and it doesn't look like you plan on passing the length along there (which you probably should do, since calculating length is a bit unoptimised), though obviously I can't tell from these snippets...

I don't know how your program works, but strings have a .c_str() to cast them back to a const char* if needed for other libraries and it's typically recommended to use the std string wherever possible to avoid the many silly mistakes associated with C-Style strings.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 10:57:56 pm
I'm using free image for my images, I actually tried using the c_str() but the library doesn't work with const chars =\

Anyway, it's not so much calculating the length really.  I have a C# program that writes this file for me, and it automatically prefixes it with the length as a 7 bit compressed integer.  So I'm really just reading that in.  Anyway, I'm going with the memcpy router.  I'll get back to you in a few minutes.
Title: Re: Memory Allocation Issue
Post by: TheDarkJay on August 19, 2009, 11:01:37 pm
No, I mean the strlen function works by doing something akin to:

char* c = whatever the string passed is;
for ( unsigned int i = 0; *c != '\0'; ++i, ++c ); return i;

Without the null terminator it will loop until it finds a random byte in the RAM that just happens to be 0. So either the freeimage has to have the length of the file name passed to it in the functions (I dunno, never used it), or it'll 99.9999% of the time get the wrong string name.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 19, 2009, 11:05:51 pm
Interesting, never had that problem though, I've been able to load up everything I've thrown at it without a problem.  Well, anyway, the memcpy route worked out.
Title: Re: Memory Allocation Issue *solved*
Post by: TheDarkJay on August 19, 2009, 11:08:13 pm
the null terminator is automatically added to strings like const char* p = "This ends in a null terminator", so if you were loading them like that it would run without the same issue.

Likewise std::string.c_str() will end with a null.

Oh, and it's not a good idea usually but you can cast away the constness of a const char*, though only do it if you know it to be safe.
Title: Re: Memory Allocation Issue
Post by: number2pencil on August 20, 2009, 01:34:53 am
Why are you using a for loop if you never evaluate or use the looping value contained in i?

If I understand what you are doing, just loop through the array while array element is less than 79 (or size of length minus one).
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 20, 2009, 01:52:11 am
Why are you using a for loop if you never evaluate or use the looping value contained in i?

If I understand what you are doing, just loop through the array while array element is less than 79 (or size of length minus one).

That's actually the forum screwing up.  For some reason it isn't showing the [ i ] in the loop.
Title: Re: Memory Allocation Issue
Post by: number2pencil on August 20, 2009, 04:38:39 am
heh, I thought that was the purpose of code tags was to allow for bb codes to be used as plain text.  Sorry :P

** Edit ** & proper indenting too
Title: Re: Memory Allocation Issue
Post by: Zaeranos on August 20, 2009, 06:23:15 am
Quote
//read in tileSet URL
   length = ReadCompressed(mapFile);
   temp = new char[length];
   for (int i = 0; i < length; i++)
   {
      fread(&temp, 1, 1, mapFile);
      if (temp[ i] == '\\')
         temp[ i] = '/';
   }
   //HVSTGFX::loadPNGTiles(temp, tempHMD.tileSet, 16, 16);
   delete temp;

I have this little algorithm for reading in a string from a file.  The string is prefixed with the length, so I read that into length first and it returns 79 in this case.  Now that's all fine, but when I call new on temp..the array is bigger than 79.  As you can imagine, this is causing problems when using the string as the directory for loading an image from.  Why the hell is it allocating more memory than it should?  Either that, or does anyone know of another way to read the string into a char array?  This is reading from a binary file btw

OK, I actually looked up what fread does in C++, and with a simple google I found this page: http://www.cplusplus.com/reference/clibrary/cstdio/fread/

If I understand correctly than the third parameter is the number of elements that has to be in the array. So my suggestion is to make the code like this:
Code: [Select]
        //read in tileSet URL
length = ReadCompressed(mapFile);
temp = new char[length];
        rLength = fread(&temp, 1, length, mapFile);
        //Check to see if it all did go correct.
        if (rLength != length) {fputs ("Reading error",stderr); exit (3);}

for (int i = 0; i < length; i++)
{

if (temp[ i] == '\\')
temp[ i] = '/';
}
//HVSTGFX::loadPNGTiles(temp, tempHMD.tileSet, 16, 16);
delete temp;

I've also added a check to see if it all went well. You should read the page to see what more error things you can get for debugging. The best thing to do is read the char array all at once.
Title: Re: Memory Allocation Issue
Post by: MG-Zero on August 20, 2009, 03:33:12 pm
Gah...everyone that reads this doesn't understand the issue.

The call to new on temp is allocating more memory than it should.  The error was not in the loop or in fread or anything, but before it when memory was allocated.  79 was passed to the array index in the allocation, and it created an array of size 94 instead. 

Anyway, the issues been solved with a little memory moving.

Contact Us | Legal | Advertise Here
2013 © ZFGC, All Rights Reserved