0

Getting a lot of errors in check2 - my paths seem to be fine but it's not returning the right files.

In GDB I'm getting a 500 error when it returns...

I feel like this is a pretty simple method, so I could benefit from a fresh set of eyes that might take a look...

Any thoughts/hints would be much appreciated!

    bool load(FILE* file, BYTE** content, size_t* length)
{
    FILE* fptr = file;
    if (fptr == NULL)
    {
        return false;
    }
    FILE* fptr_cpy = fptr;
    // Initialize location in memory    
    BYTE* content_ptr;
     // initialize message and its length
    *content = NULL;
    *length = 0;
    int cont_len = 0;

    // read message 
    for (BYTE c = fgetc(fptr); c != EOF; c = fgetc(fptr))
    {
        // read length of file
        cont_len++;
    }

    content_ptr = malloc(cont_len);
    if (content_ptr == NULL)
    {
        return false;
    }
    BYTE cont_bfr[cont_len];
    int i = 0;
    for (BYTE c = fgetc(fptr_cpy); c != EOF; c = fgetc(fptr_cpy), i++)
    {
        cont_bfr[i] = c;
    }

    memcpy(content_ptr, cont_bfr, cont_len);
    *content = content_ptr;
    *length = cont_len;
    if(*content != 0 && *length != 0)
    {
        return true;
    }
    return false;
}

1 Answer 1

1

It seems you're dealing with fptr_cpy thinking that it would be another file and thus it wouldn't be at it's end after you read it all the way to EOF with your previous loop, but that's not true. Both fptr and fptr will point to the same file and that file is already at it's end when you reach your second loop.

fseek() back all the way to the beginning of the file before the following loop and this load() function will start to work properly:

   // fseek() all the way back here  
    for (BYTE c = fgetc(fptr_cpy); c != EOF; c = fgetc(fptr_cpy), i++)
        {
            cont_bfr[i] = c;
        }

Second, use int c = fgetc(...) instead of BYTE c = fgetc(), otherwise you'll get false EOF detections:

4.1 Using fgetc(), etc. incorrectly

The fgetc(), getc() and getchar() functions all return back an integer value. For example, the prototype of fgetc() is:

int fgetc( FILE * );

Sometimes this integer value is really a simple character, but there is one very important case where the return value is not a character!

What is this value? EOF A common misconception of students is that files have a special EOF character at the end. There is no special character stored at the end of a file. EOF is an integer error code returned by a function. Here is the wrong way to use fgetc():

int count_line_size( FILE * fp )
{
char ch;
int cnt = 0;

while( (ch = fgetc(fp)) != EOF && ch != '\n')
cnt++;
return cnt;
}

What is wrong with this? The problem occurs in the condition of the while > loop. To illustrate, here is the loop rewritten to show what C will do behind the scenes.

while( (int) ( ch = (char) fgetc(fp) ) != EOF && ch != '\n')
cnt++;

The return value of fgetc(fp) is cast to char to store the result into ch. Then the value of ch must be cast back to an int to compare it with EOF. So what? Casting an int value to a char and then back to an int may not give back the original int value. This means in the example above that if fgetc() returns back the EOF value, the casting may change the value so that the comparison later with EOF would be false.

What is the solution? Make the ch variable an int as below:

int count_line_size( FILE * fp )
{
int ch;
int cnt = 0;

while( (ch = fgetc(fp)) != EOF && ch != '\n')
cnt++;
return cnt;
}

Now the only hidden cast is in the second comparison.

while( (ch = fgetc(fp)) != EOF && ch != ((int) '\n') )
cnt++;

This cast has no harmful effects at all! So, the moral of all this is: always use an int variable to store the result of the fgetc(), getc() and getchar().

http://www.drpaulcarter.com/cs/common-c-errors.php#4.1

PS: This is me trying to keep most part of your code intact, but I don't like the design very much. Nevertheless, it's not my place to judge anyone's design.

2
  • Thanks, will look into it--what would you do with the design? I'm sure my design is suboptimal, but online there's no one to compare against without 'cheating'
    – borker
    Commented Nov 8, 2016 at 21:27
  • The main thing I would do is use just one loop instead of two and perform the length* tracking and content* filling more or less at the same time. But don't bother with this yet, try to make your code work first. You're very close to an answer that will work with your design as is. Commented Nov 8, 2016 at 21:48

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .