Archived

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

char memory leak?

This topic is 5516 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
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
Why don''t you try: number = atoi(string);
instead of your number = atoi((const char *)string); ?

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
quote:
Original post by sark
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.
What I meant was that he shouldn''t concern himself with increase in file size and that C++ doesn''t have any disadvantages anymore, when compared to C (situation was different 10 years ago). Not that C++ was a new and hot language.

Share this post


Link to post
Share on other sites
quote:
Original post by civguy
push_back for strings? Sounds pretty tedious to me, when there''s operator+=

you can see i''m using vectors much more often than strings.

Share this post


Link to post
Share on other sites
What I meant with those comments was that, since C++ compiles much more strictly (at least with my compiler), I''ll have to go back and change some of my more sloppy sections of code in that program. Since this is a makeshift program anyway, and is getting rather long already (I had hoped to be finished with it by now and to get on with the more important thing: the actual game engine), I don''t want to go back and do all that. Thus the extra typing.

Also, I''ll have to familiarize my self with the std::string library; thus far I''ve only used C-strings and been quite content.

Twilight Dragon

Share this post


Link to post
Share on other sites