Skip to main content
GameDev.net gamedev.net
🔒 Locked

char memory leak?

Started by TDragon Dec 2, 2002 at 5:04 PM 11 replies 3.1k views
Original Post
TDragon
TDragon
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
Xai
Xai
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;
  }

Raduprv
Raduprv
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?
DerekSaw
DerekSaw
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);
}


"after many years of singularity, i'm still searching n the event horizon" o
niyaw
niyaw
you''re using a fixed-length string to read an unknown number of bytes from the file. use std::string and push_back instead.
GdWiki
TDragon
TDragon
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
civguy
civguy
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+=
sark
sark
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.
civguy
civguy
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.

niyaw
niyaw
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.
GdWiki
TDragon
TDragon
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

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.