0

I have a program that records data from a serial port. Every so often, I want to split up files such that the data logs don't become very large. The problem is, after I recreate the FILE* and try to write into it, the program crashes. No compiler errors/warnings before hand also...

The program does create one log for the first time interval, but once it's time to create a new data log, it crashes at the fwrite.

First off, initializations/declarations.

char * DATA_DIR = "C:\DATA";
sprintf(path,"%s%s%s",DATA_DIR,curtime,".log"); //curtime is just the current time in a string
FILE * DATA_LOG = fopen(path, "wb+");   

And later on in a while loop

if(((CURRENT_TIME-PREVIOUS_TIME) > (SEC_IN_MINUTE * MINUTE_CHUNKS) ) && (MINUTE_CHUNKS != 0) && FIRST_TIME == 0) //all this does is just checks if its time to make a new file
{
    fclose(DATA_LOG); //end the current fileread

    char * path; 
    char curtime[16];

    //gets the current time and saves it to a file name
    sprintf(curtime , "%s" , currentDateTime());
    sprintf(path,"%s%s%s",DATA_DIR,curtime,".log");

    DATA_LOG = fopen(path, "wb+"); //open the new file

    //just some logic (not relevant to problem)
    PREVIOUS_TIME = CURRENT_TIME; 
    newDirFlag = 1;
}
fwrite(cdata , sizeof(char) , numChars , DATA_LOG); //crashes here. cdata, sizeof, and numChars don't change values

Any ideas why is this happening? I'm stumped.

user2619824
  • 478
  • 1
  • 5
  • 20

2 Answers2

1

Couple of problems, path has no memory allocated (you're writing stuff to some random memory address which is bad). You also should check the return values of fwrite fopen for errors. If there is one use perror so you know what the problem is. It's likely the fopen is failing or you're corrupting your stack by writing to path.

Also use snprintf it's much safter than just sprintf which is vulnerable to buffer overflow.

EDIT: just saw your comment that it's c++. Why not use std::string and fstream instead? They are much safer than what you're currently doing (and probably easier).

Jesus Ramos
  • 22,940
  • 10
  • 58
  • 88
  • Apologies. I don't have any formal experience in C++ (only Java). I'm changing code written by a previous intern, so I didn't know about fstream since he didn't use it in this code. Are you saying that instead of char * path; I can do something like std:string path? I wouldn't need to worry about the random memory address issue? – user2619824 Aug 20 '13 at 22:53
  • Yes, `std::string` will handle allocating memory for the string and such so you don't have to worry about making `char path[512];` or something to hold the path. You can always pass the underlying string by calling `my_string.c_str()` if you still want to use `fopen` and such. – Jesus Ramos Aug 20 '13 at 22:57
0

Your MAIN problem is that char * path; has no memory assigned to it. This means that you are writing to some RANDOM [1] location in memory.

I would suggest that you use char path[PATH_MAX]; - that way you don't have to worry about allocating and later deallocating the storage for your path.

Alternatively, you could use:

stringstream ss;

ss << DATA_DIR << currentDateTime() << ".log";
string path = ss.str();
fopen(path.c_str(), "wb+")

which is a more C++ style solution.

[1] By random, I don't mean truly a random number, but some unknown value that happens to be in that location on the stack. It is almost always NOT a good place to store a string.

Mats Petersson
  • 126,704
  • 14
  • 140
  • 227
  • I wound up doing this. Ultimately I found out that my problem was that my path string somehow did change and that messed up my fopen. I learned a lot from this post/googling from it. – user2619824 Aug 21 '13 at 17:35