Jump to content
  • Advertisement

Archived

This topic is now archived and is closed to further replies.

TDragon

char memory leak?

This topic is 5850 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have a function that will read a string from a file and convert it to a number (using fread() and atoi()). Problem is, I think there''s a memory leak somewhere, because one of my global integers keeps getting overwritten. Could someone look at my code and tell me what I should change? The idea is to be able to assign an integer to the return value of the function, thus:
int value = ReadNumber(afile, 96); 
such that when the function reaches a character of value 96 (or whatever is passed) it converts what it has into a number and returns the number. I think the problem is actually happening at the calling of the atoi() function.
  
int ReadNumber(FILE *thefile, char stopval)
{
 int number, i, exloop;
 char extra;
 char string[5];

 i = 0;
 exloop = 0;
 while(!exloop)
 {
  fread(&extra, sizeof(char), 1, thefile);
  if(char1 == stopval)
  {
   exloop = 1;
  }
  else
  {
   string[i] = char1;
  }
  i++;
 }
 string[i] = NULL;
 number = atoi((const char *)string);
 
 
 return number;
}
  
Twilight Dragon

Share this post


Link to post
Share on other sites
Advertisement
It seems you may be exceeding your 5 character string buffer. Here''s my slightly changed version


  
#define MAX_NUM_LEN 33

int ReadNumber(FILE *theFile, char stopVal)
{
int number;
int i;
int stopFound = 0;
char tempChar;
char string[MAX_NUM_LEN + 1];

for(i = 0; (i < MAX_NUM_LEN) && !stopFound ; ++i)
{
fread(&tempChar, sizeof(tempChar), 1, theFile);
if(tempChar== stopVal)
{
stopFound = 1;
string[i] = ''\0''; // null terminate the string

}
else
{
string[i] = char1;
}
}

if(!stopFound)
string[MAX_NUM_LEN] = NULL;

number = atoi((const char *)string);

return number;
}

Share this post


Link to post
Share on other sites
Heh - had to go back 6 pages to dig this up.
Oh well.

The method you gave still does the exact same thing. Does anyone else know whats going on?

Twilight Dragon

Share this post


Link to post
Share on other sites
Besides for making that string longer (try some insane value, like 1 MB, to be sure), I don''t see any other problem. Are you SURE it''s that function that trash your global integers?

Share this post


Link to post
Share on other sites
Another changed version from Xai:

    
#define MAX_NUM_LEN 33
int ReadNumber(FILE *theFile, char stopVal)
{
char str[MAX_NUM_LEN + 1];
int i;
int tempChar;

for (i = 0; i < MAX_NUM_LEN; ++i)
{
tempChar = fgetc(theFile);
if (tempChar == EOF) // end of file

break;

if (tempChar == stopVal)
{
str[i] = 0;
break;
}

str[i] = tempChar;
}

str[i] = 0;

return atoi(str);
}


Share this post


Link to post
Share on other sites
you''re using a fixed-length string to read an unknown number of bytes from the file. use std::string and push_back instead.

Share this post


Link to post
Share on other sites
Raduprv: I''ve tried a limit of 5,000 (a 5,000 digit number is more than insane); since none of the numbers I''m reading are more than 3 digits I don''t think that''s the problem. I have narrowed it down to that function for certain; funny thing is that it only happens a certain time the function is called, although it gets called many, many times total. Another funny thing is that the global variable (a member of a global array of int''s, to be precise) gets overwritten with that number; it seems (however unlikely) to be happening during the call to atoi().

DerekSaw: I spotted two changes in the code: a check for EOF and a change to fgetc(). I''ll try it, but since I''m certain that I''m not reaching EOF (couldn''t possibly) and I don''t think there''s any difference between fgetc(theFile) and fread(&char, 1, sizeof(char), theFile), I''m pretty certain it won''t work.

niyaw: Since the code is designed not to let the reading get past the length of the string, then the unknown number of bytes shouldn''t be a problem. Remember that a C-Style String is really just an array of chars (at least in this case). I''m not even working with pointers in this case. However, I might have to end up going to std::string stuff if nothing else can be done. But that means switching over to C++, which means more strict compiling (), more typing, and a bigger file size. Oh well.

Twilight Dragon

Share this post


Link to post
Share on other sites
quote:
Original post by TDragon
But that means switching over to C++, which means more strict compiling (), more typing, and a bigger file size.
Hmm, welcome to 21st century. Anyway, "more typing"? C++ has more expression power than C. Your function could be done like this:

  
int ReadNumber(ifstream& in, char stopVal) {
string buf;
char readCh;

while (!in.eof()) {
in >> readCh;
if (in.eof() || readCh == stopVal) break;
buf += readCh;
}
return atoi(buf.c_str());
}

I don''t need to count the characters to say that it''s a lot smaller
quote:
use std::string and push_back instead.
push_back for strings? Sounds pretty tedious to me, when there''s operator+=

Share this post


Link to post
Share on other sites
quote:
Original post by civguy
Hmm, welcome to 21st century.


C++? 21st Century? C++ is not a modern language... and it shows. I dont mean to start a flame war, but any language where you have to use forward declarations doesnt classify as a "21st century language" to me.

Share this post


Link to post
Share on other sites

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

GameDev.net is your game development community. Create an account for your GameDev Portfolio and participate in the largest developer community in the games industry.

Sign me up!