4 replies to this topic

[Reedbeta]

I'm trying to write code to parse a MIDI file, which consists of chunks. I'm trying to use this loop to filter out all the chunks I'm not interested in:

// Read each chunk in turn and process if it's a track chunk
FILE *fp;
// ...open the file, blah blah...

// Each chunk consists of this header, followed by some data 
struct midi_chunk_t
{
	union {
		char c_id[4];		// Four-character chunk identification
		unsigned int id;	// As a 32-bit integer for easy comparison
	};
	unsigned int length;	// Length of following data in bytes (BIG ENDIAN)
};
midi_chunk_t chunk;

while (!feof(fp))
{
	fread(&chunk, sizeof(chunk), 1, fp);
	chunk.length = swapbytes(chunk.length);
	if (chunk.id == MIDI_TRACK_CHUNK)
	{
		processTrack(fp, chunk.length);
	}
	else
	{
		fseek(fp, chunk.length, SEEK_CUR);
	}
}

For some reason, feof() never seems to trigger the end of the loop. When I run it on a file, it correctly reads the chunks that actually exist, and then continues past the end of the file, reading garbage until I press Ctrl+C.

I can, however, correctly detect the end of the file by checking the return value of the fread() call. It returns 0 when there is no more data to be read.

I don't understand why feof() isn't returning true. I compiled this both with MSVC 7.1 and with g++, and the behavior is the same, so I figure it's a problem with my code and not the compiler or the libraries. Any ideas?

[bladder]

feof returns a non-zero value after a read (or write?) operation that goes past the file. However, the calling of fseek (or rewind, setpos and clearerr) clears the eof bit that is used to determine if the fp has gone past the end of file. So I'm guessing if you get rid of that fseek then it'll work. Apparently it's legal to seek past the end of the file without setting off any alarms.

I suppose changing it to:

while (!feof(fp))
{
	fread(&chunk, sizeof(chunk), 1, fp);
	chunk.length = swapbytes(chunk.length);
	if (chunk.id == MIDI_TRACK_CHUNK)
	{
		processTrack(fp, chunk.length);
	}
	else
	{
		char* dummy = new char[chunk.length];
		fread(dummy, sizeof(chunk), 1, fp);
	}
}

[Reedbeta]

Hmm, good call bladder. That behavior seems silly to me, but if it's what the spec says, how can I argue? Anyway, I didn't want to allocate memory and read in that extra data, so I just replaced the while condition with 'ftell(fp) < file_length'.

[Jare]

Reedbeta said:

I just replaced the while condition with 'ftell(fp) < file_length'.

In general, it's good advice to be very conservative with file operations. In this case, you can just implement a file position counter yourself and avoid extra ftell()'s. You normally won't have performance problems, but you might be surprised; multithread safety for example can make many operations much more expensive than originally expected.

[Reedbeta]

Thanks for the advice, but that loop only goes round once for each chunk in the file, and there are just a handful of chunks.