-1

hope someone can assist. I had this program running perfectly, moved a few lines of code from one function to another and it all fell apart.

I've included a snippet below from the top of the function until segfault. It happily outputs "did we get here?" but not the next statement, I've spent so many hours trying to figure this that I can't remember the working build I had to begin with

It (at least the section below) is supposed to copy a whole textfile to a string

Morals of the story: working code is better than 'correct' code, always copy working code before you try to tweak it.

void validateFile(FILE* file, char** menuStore, char** submenuStore)
{

    char* temp = NULL;
    size_t size;
    boolean flag = true;
    char first;

    /*Loop Counter*/
    int i;

    fseek(file, 0, SEEK_END);
    size = ftell(file) * sizeof(char);
    fseek(file, 0, SEEK_SET);

    if ((temp = malloc(size)) == NULL)
    {
        printf("\nUnable to allocate Memory, Program exiting");
        exit(EXIT_FAILURE);
    } else
    {
        for (i = 0; i < (size / sizeof(char)); i++)
        {
            temp[i] = fgetc(file);
        }

        printf("\n did we get here?");
        printf("\nFile loaded, validating...");
Thomas Dickey
  • 51,086
  • 7
  • 70
  • 105
Xeke
  • 21
  • 2
  • 2
    `sizeof(char)` is always 1 – David Ranieri Aug 13 '14 at 11:54
  • 1
    Put newlines *last* in the string to print. Remember that output to `stdout` is line-buffered by default, so whatever happens after that second `printf` you will not see the text until you print a newline (or explicitly flush the buffer). – Some programmer dude Aug 13 '14 at 11:55
  • 2
    As for crashes, *always* run your crashing program in a debugger. The debugger will stop at the location of the crash, and let you examine and walk up the function call stack. – Some programmer dude Aug 13 '14 at 11:56
  • @JoachimPileborg Thanks for that, I would have thought one of my tutors would have mentioned that at some point over the last 4 years... I'll have a look at the stuff after that point now and keep digging. – Xeke Aug 13 '14 at 12:00

1 Answers1

-1

While reading from a file you should not use loop like this-

for (i = 0; i < (size / sizeof(char)); i++)
{
    temp[i] = fgetc(file);
}

You should check for EOF condition while reading from a file-

i=0;
while((temp[i]=fgetc(file))!=EOF)
     i++;

and make end of the string to \0. That is the better way to do.

temp[i]='\0';
Sathish
  • 3,740
  • 1
  • 17
  • 28
  • Thanks Sathish, Joachim made me realise where the error was occurring, i'm calling strcpy() a non-NUL-terminated string (a little further along in the code) thanks for all the help guys :) – Xeke Aug 13 '14 at 12:19
  • [About `while(!feof(file))`...](https://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong) – David Frye Aug 13 '14 at 12:20
  • @DavidFrye I just read that earlier today actually. If I'm already going to be working with exact file sizes/string lengths for my malloc, I might as well use a for() (just have to add a +1 to length and add the /0 before I strcpy) – Xeke Aug 13 '14 at 12:23
  • I'm not saying that the technique is _always_ wrong (though the answer itself is), I just figured I'd post it, as it's relevant (and I just read it today as well!). – David Frye Aug 13 '14 at 12:25
  • This is **not** the correct way to check for `EOF`. `fgetc()` returns either an `unsigned char` or `EOF`. This method folds the `EOF` into one of the 256 values stored in `temp[i]`. Instead `int ch; while((ch=fgetc(file))!=EOF) { temp[i] = ch; ...`. – chux - Reinstate Monica Aug 16 '14 at 02:40