Thread: Problem reading file.

>unsigned char* ReadVPfile
>char *filename
>FILE * file
Um...pick a style and go with it. Inconsistency makes your code harder to read. You've already covered the board in pointer declaration styles, and while that certainly stops all controversy, it also effects the clarity of your code. In fact, it looks like several people write it as a collaborative effort but couldn't make up their minds.

>return false;
false is not a pointer to unsigned char. If you want to return a null pointer then say NULL or 0, k? This is even more important in C89 where false is not a keyword.

>fseek(file, 0, SEEK_END);
>vpfileSize = ftell(file);
This is not a reliable way to get the size of a file. The best way is to read it. Start at the beginning and read to the end. While you're at it you can resize vpfileText to hold the new data. At EOF, you can be reasonably sure that everything is peachy.

>//allocate memory
Unless you're using C99, this is a nonstandard extension. I know you're not using C99 because new isn't an allocation method in C99.

>unsigned char * vpfileText=new unsigned char[vpfileSize];
This is not C, it's C++. 1) new is not a way to allocate memory in C and 2) C89 does not allow declarations except at the beginning of a block. C99 fixes this, but since you're obviously compiling as C++, I can safely assume you didn't intend C99.

>fread(vpfileText, 1, vpfileSize, file);
>return vpfileText;
If you intend to use vpfileText as a string then you'll get garbage unless you append a null character at the end:

Code:

unsigned char *ReadVPfile(char *filename)
{
  FILE *file;
  long vpfileSize;
  unsigned char *vpfileText;

  file = fopen(filename, "rb");
  if(!file)
    return NULL;

  fseek(file, 0, SEEK_END);
  vpfileSize = ftell(file);

  rewind(file);

  vpfileText = malloc(vpfileSize + 1);
  if(!vpfileText) {
    fclose(file);
    return NULL;
  }

  fread(vpfileText, 1, vpfileSize, file);
  vpfileText[vpfileSize] = '\0';
  fclose(file);

  return vpfileText;
}

You also forgot to close the file before the function returns. And error checking for fread would be a good thing.

And the master responds. . I've been sitting here staring at his code

Code:

//allocate memory
	unsigned char * vpfileText=new unsigned char[vpfileSize]

trying to rack my brain about this line as to what he was trying to do. I've never done c++ so I wouldn't know. I ignored the OP coding style, but that had me. Then again i've only recently finished C and now learning to apply it, so this was a good thread to 'C'.

[edit] is it really necessary to declare char * to unsigned? [/edit]

>In which cases will this way fail to give you the correct size?
In any case. The C standard states explicitly that fseek calls on a binary stream with SEEK_END as the last argument need not be meaningfully supported.

>Can you quote the relevant portion of the standard that relates to that Prelude?
Sure:

Code:

7.19.9.2 The fseek function

Synopsis

    #include <stdio.h>
    int fseek(FILE *stream, long int offset, int whence);

Description

The fseek function sets the file position indicator for the stream pointed to by stream.
If a read or write error occurs, the error indicator for the stream is set and fseek fails.

For a binary stream, the new position, measured in characters from the beginning of the
file, is obtained by adding offset to the position specified by whence. The specified
position is the beginning of the file if whence is SEEK_SET, the current value of the file
position indicator if SEEK_CUR, or end-of-file if SEEK_END. A binary stream need not
meaningfully support fseek calls with a whence value of SEEK_END.

Originally Posted by Prelude

This is not a reliable way to get the size of a file. The best way is to read it. Start at the beginning and read to the end.

Just curious, is stat() not reliable?

Originally Posted by itsme86

Just curious, is stat() not reliable?

I don't believe
stat is standard. I think prelude was trying to get across the more standard compliant way.

Well, it's POSIX standard at least But I guess you're right.

>Just curious, is stat() not reliable?
It depends on what you want and what else is running. If another process is writing to the file then stat won't work very well. The biggest problem with stat is that it's not portable past POSIX systems. If that isn't a problem then by all means use it as the result is accurate.