Crashing with cstdio file class

J

John Douglass

I'm fairly new to doing involved file i/o and I came across something
weird with a program I'm writing (modifying MIDI data, if it makes any
difference). With some input files, when I modify data and then output
to a new file, the program will crash. The catch is that when I open up
the output file to see what the program managed to write, it always
makes it to an offset of xFFF.

For example, with one file it crashed at 2FFF, another at 1FFF, and
another at 6FFF. The actual data at those addresses have no connection
to one another, so I can't really find a pattern. I'm wondering if
there's some exception to file writing with cstdio that would cause
something like this.

Thanks.
 
I

Ian

John said:
I'm fairly new to doing involved file i/o and I came across something
weird with a program I'm writing (modifying MIDI data, if it makes any
difference). With some input files, when I modify data and then output
to a new file, the program will crash. The catch is that when I open up
the output file to see what the program managed to write, it always
makes it to an offset of xFFF.
Could be the offset is a multiple of a buffer size, more detail required
for anyone to help, post some code.

Ian
 
J

John Douglass

Ian said:
Could be the offset is a multiple of a buffer size, more detail required
for anyone to help, post some code.

Ian

Okay, here is the main bulk of the code:
FILE * pInFile;
FILE * pOutFile;

pInFile = fopen(s_inFile, "rb");
pOutFile = fopen(s_outFile, "wb");

// bool flags
bool bFlag = 0;

do {
// read a byte
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if we have a note-on status message
if (szBuffer[0] == szNoteOn[0]) {
// write the byte
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// prepare for while loop
bFlag = 0;

// deal with notes marked after note_on message
do {
// convert note
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 95
if (strcmp(&szBuffer[0], &szVelTarget[0]) >= 0) {
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}

// write the velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if the next byte is a null, write and go back through the loop
// if the next byte is a all notes off status message, write and
exit the loop
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szAllNotesOff[0]) {
bFlag = 1;
}
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
while (!bFlag);

// the next byte should be a note_off status message (0x89)
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// prepare for while loop
bFlag = 0;

// deal with note messages after status message
do {
// convert note and write
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 80
if (strcmp(&szBuffer[0], &szVelTargetOff[0]) >= 0) {
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}
// write velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// this byte should be a null, so write it
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if we're back at a note_on message, exit loop and push the byte
back into the stream
// if we're at a 0xFF message, we're almost at the end of the file,
so write the byte and exit the loop
// otherwise, push the byte back into the stream and go through the
loop again
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szNoteOn[0]) {
bFlag = 1;
ungetc(szBuffer[0], pInFile);
}
else if (szBuffer[0] == szEOFMarker[0]) {
bFlag = 1;
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
ungetc(szBuffer[0], pInFile);
}
else {
ungetc(szBuffer[0], pInFile);
}
}
while (!bFlag);
}
// if we don't have a note-on, go back through the loop
else {
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
}
// exit when we get to the end of the file
while (dwRead > 0);

fclose(pInFile);
fclose(pOutFile);

--------------------

the "gmToDKFH()" function just takes a byte from the buffer and changes
it according to the midi standard I'm converting to.

Here are some of the relevent strings/chars I'm comparing in the program:

// bytes read
size_t dwRead;

// single char file buffer
char szBuffer[2];
szBuffer[1] = 0x00;

// constants
char szNoteOn[2];
sprintf(&szNoteOn[0], "%c", 0x99);
szNoteOn[1] = 0x00;

char szNoteOnMin[2];
sprintf(&szNoteOnMin[0], "%c", MIDI_CMD_NOTE_ON);
szNoteOnMin[1] = 0x00;

char szNoteOnMax[2];
sprintf(&szNoteOnMax[0], "%c", 0x9F);
szNoteOnMax[1] = 0x00;

char szNoteOffMin[2];
sprintf(&szNoteOffMin[0], "%c", MIDI_CMD_NOTE_OFF);
szNoteOffMin[1] = 0x00;

char szNoteOffMax[2];
sprintf(&szNoteOffMax[0], "%c", 0x8F);
szNoteOffMax[1] = 0x00;

char szAllNotesOff[2];
sprintf(&szAllNotesOff[0], "%c", MIDI_CTL_ALL_SOUNDS_OFF);
szAllNotesOff[1] = 0x00;

// velocity 95
char szVelTarget[2];
sprintf(&szVelTarget[0], "%c", 0x5f);
szVelTarget[1] = 0x00;

char szVelTargetOff[2];
sprintf(&szVelTargetOff[0], "%c", 0x50);
szVelTargetOff[1] = 0x00;

// hihat constants
char szOpenHat[2];
sprintf(&szOpenHat[0], "%c", 0x43);
szOpenHat[1] = 0x00;

char szClosedHat[2];
sprintf(&szClosedHat[0], "%c", 0x3e);
szClosedHat[1] = 0x00;

// 0xFF eof marker for midi
char szEOFMarker[2];
sprintf(&szEOFMarker[0], "%c", 0xff);
szEOFMarker[1] = 0x00;

// null target
char szNull[2];
sprintf(&szNull[0], "%c", 0x00);
szNull[1] = 0x00;
 
P

puzzlecracker

John said:
Ian said:
Could be the offset is a multiple of a buffer size, more detail required
for anyone to help, post some code.

Ian

Okay, here is the main bulk of the code:
FILE * pInFile;
FILE * pOutFile;

pInFile = fopen(s_inFile, "rb");
pOutFile = fopen(s_outFile, "wb");

// bool flags
bool bFlag = 0;

do {
// read a byte
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if we have a note-on status message
if (szBuffer[0] == szNoteOn[0]) {
// write the byte
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// prepare for while loop
bFlag = 0;

// deal with notes marked after note_on message
do {
// convert note
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 95
if (strcmp(&szBuffer[0], &szVelTarget[0]) >= 0) {
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}

// write the velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if the next byte is a null, write and go back through the loop
// if the next byte is a all notes off status message, write and
exit the loop
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szAllNotesOff[0]) {
bFlag = 1;
}
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
while (!bFlag);

// the next byte should be a note_off status message (0x89)
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// prepare for while loop
bFlag = 0;

// deal with note messages after status message
do {
// convert note and write
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 80
if (strcmp(&szBuffer[0], &szVelTargetOff[0]) >= 0) {
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}
// write velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// this byte should be a null, so write it
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if we're back at a note_on message, exit loop and push the byte
back into the stream
// if we're at a 0xFF message, we're almost at the end of the file,
so write the byte and exit the loop
// otherwise, push the byte back into the stream and go through the
loop again
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szNoteOn[0]) {
bFlag = 1;
ungetc(szBuffer[0], pInFile);
}
else if (szBuffer[0] == szEOFMarker[0]) {
bFlag = 1;
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
ungetc(szBuffer[0], pInFile);
}
else {
ungetc(szBuffer[0], pInFile);
}
}
while (!bFlag);
}
// if we don't have a note-on, go back through the loop
else {
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
}
// exit when we get to the end of the file
while (dwRead > 0);

fclose(pInFile);
fclose(pOutFile);

--------------------

the "gmToDKFH()" function just takes a byte from the buffer and changes
it according to the midi standard I'm converting to.

Here are some of the relevent strings/chars I'm comparing in the program:

// bytes read
size_t dwRead;

// single char file buffer
char szBuffer[2];
szBuffer[1] = 0x00;

// constants
char szNoteOn[2];
sprintf(&szNoteOn[0], "%c", 0x99);
szNoteOn[1] = 0x00;

char szNoteOnMin[2];
sprintf(&szNoteOnMin[0], "%c", MIDI_CMD_NOTE_ON);
szNoteOnMin[1] = 0x00;

char szNoteOnMax[2];
sprintf(&szNoteOnMax[0], "%c", 0x9F);
szNoteOnMax[1] = 0x00;

char szNoteOffMin[2];
sprintf(&szNoteOffMin[0], "%c", MIDI_CMD_NOTE_OFF);
szNoteOffMin[1] = 0x00;

char szNoteOffMax[2];
sprintf(&szNoteOffMax[0], "%c", 0x8F);
szNoteOffMax[1] = 0x00;

char szAllNotesOff[2];
sprintf(&szAllNotesOff[0], "%c", MIDI_CTL_ALL_SOUNDS_OFF);
szAllNotesOff[1] = 0x00;

// velocity 95
char szVelTarget[2];
sprintf(&szVelTarget[0], "%c", 0x5f);
szVelTarget[1] = 0x00;

char szVelTargetOff[2];
sprintf(&szVelTargetOff[0], "%c", 0x50);
szVelTargetOff[1] = 0x00;

// hihat constants
char szOpenHat[2];
sprintf(&szOpenHat[0], "%c", 0x43);
szOpenHat[1] = 0x00;

char szClosedHat[2];
sprintf(&szClosedHat[0], "%c", 0x3e);
szClosedHat[1] = 0x00;

// 0xFF eof marker for midi
char szEOFMarker[2];
sprintf(&szEOFMarker[0], "%c", 0xff);
szEOFMarker[1] = 0x00;

// null target
char szNull[2];
sprintf(&szNull[0], "%c", 0x00);
szNull[1] = 0x00;

-------------------

If any explanation or further code is needed, I'd be happy to provide.

Thanks


This is a rather bulky size of code, and it will take me an hour to
find a bug. I charge 50$/hour, but willing to make an exception, and it
will cost you $40.99. Respond if interested.
 
I

Ian

John said:
Okay, here is the main bulk of the code:

What happens if fread return 0? You don't test.

How does it crash?

Think about replacing each commented bit of code with a function, it
will make things clearer.

Ian
 
J

Jack Klein

On Sun, 02 Oct 2005 18:10:41 -0400, John Douglass


I haven't looked at your code in detail, and I don't know what your
particular problem is, but I thought I would point out a few things.
Okay, here is the main bulk of the code:
FILE * pInFile;
FILE * pOutFile;

pInFile = fopen(s_inFile, "rb");
pOutFile = fopen(s_outFile, "wb");

It's a very bad idea to assume that fopen() succeeded. If it did not,
you will be passing a null pointer to other stdio functions causing
undefined behavior.

if ((pInFile = fopen(s_inFile, "rb")) == NULL)
{
/* error handling */
}
// bool flags
bool bFlag = 0;

do {
// read a byte
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

Just a few comments:

The expression '&szBuffer[0]' is exactly equivalent in a function call
to 'szBuffer', assuming that 'szBuffer' is defined as an array of some
type. Each expression evaluates to the address of szBuffer[0], and
has the type pointer to whatever type szBuffer is an array of.

Also note that fgetc() and fputc() work just fine for binary files,
and are a lot more concise than fread() and fwrite(). You could
translate the line above to:

szBuffer[0] = fgetc(pInFile);

Then you can check szBuffer[0] for EOF.
// if we have a note-on status message
if (szBuffer[0] == szNoteOn[0]) {
// write the byte
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

Likewise here:

if (EOF == fputc(szBuffer[0], pOutFile)
{
/* error handling */
}
// prepare for while loop
bFlag = 0;

// deal with notes marked after note_on message
do {
// convert note
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 95
if (strcmp(&szBuffer[0], &szVelTarget[0]) >= 0) {

You only ever read one character into szBuffer[0]. It most certainly
is not likely to be a '\0' terminated C string, despite what you have
named it. You can't just pass any pointer to character(s) to
strcmp(), it must be a valid null terminated C string.
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}

// write the velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if the next byte is a null, write and go back through the loop
// if the next byte is a all notes off status message, write and
exit the loop
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szAllNotesOff[0]) {
bFlag = 1;
}
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
while (!bFlag);

// the next byte should be a note_off status message (0x89)
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// prepare for while loop
bFlag = 0;

// deal with note messages after status message
do {
// convert note and write
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
gmToDKFH(&szBuffer[0]);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// convert velocity
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

// if velocity is >= 80
if (strcmp(&szBuffer[0], &szVelTargetOff[0]) >= 0) {
// if we're not going to change the hihats later, or this isn't a hihat
if ((!b_useHihat) || (strcmp(&szBuffer[0], &szOpenHat[0])) &&
(strcmp(&szBuffer[0], &szClosedHat[0]))) {
// set new velocity
szBuffer[0] = (char) 0x7f;
}
}
// write velocity
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// this byte should be a null, so write it
dwRead = fread(&szBuffer[0], 1, 1, pInFile);
fwrite(&szBuffer[0], 1, dwRead, pOutFile);

// if we're back at a note_on message, exit loop and push the byte
back into the stream
// if we're at a 0xFF message, we're almost at the end of the file,
so write the byte and exit the loop
// otherwise, push the byte back into the stream and go through the
loop again
dwRead = fread(&szBuffer[0], 1, 1, pInFile);

if (szBuffer[0] == szNoteOn[0]) {
bFlag = 1;
ungetc(szBuffer[0], pInFile);
}
else if (szBuffer[0] == szEOFMarker[0]) {
bFlag = 1;
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
ungetc(szBuffer[0], pInFile);
}
else {
ungetc(szBuffer[0], pInFile);
}
}
while (!bFlag);
}
// if we don't have a note-on, go back through the loop
else {
fwrite(&szBuffer[0], 1, dwRead, pOutFile);
}
}
// exit when we get to the end of the file
while (dwRead > 0);

fclose(pInFile);
fclose(pOutFile);

--------------------

the "gmToDKFH()" function just takes a byte from the buffer and changes
it according to the midi standard I'm converting to.

Here are some of the relevent strings/chars I'm comparing in the program:

// bytes read
size_t dwRead;

Oh, forget the remarks I made about strcmp() up above, now that I have
seen this. You seem to be very unfamiliar with how characters and C
style strings work. All the places where you are using strcmp() you
could be doing single character compares. But there are other issues.
// single char file buffer
char szBuffer[2];
szBuffer[1] = 0x00;

You don't need this at all, especially if you use fgetc() and fputc()
in place of fread() and fwrite().
// constants
char szNoteOn[2];
sprintf(&szNoteOn[0], "%c", 0x99);
szNoteOn[1] = 0x00;

Are you aware that you could have written this:

char szNoteOn[2] = { 0x99 };

....and not needed the sprintf() function call at all? Or that if you
do use sprintf(), it will add the '\0' terminator automatically?

I am assuming that you are programming for Windows, and in that case
0x99 is not a valid value for a plain (signed) char. You should be
using unsigned char for all the values read and written from the
binary files, and for the values you compare them to.
char szNoteOnMin[2];
sprintf(&szNoteOnMin[0], "%c", MIDI_CMD_NOTE_ON);
szNoteOnMin[1] = 0x00;

char szNoteOnMax[2];
sprintf(&szNoteOnMax[0], "%c", 0x9F);
szNoteOnMax[1] = 0x00;

char szNoteOffMin[2];
sprintf(&szNoteOffMin[0], "%c", MIDI_CMD_NOTE_OFF);
szNoteOffMin[1] = 0x00;

char szNoteOffMax[2];
sprintf(&szNoteOffMax[0], "%c", 0x8F);
szNoteOffMax[1] = 0x00;

char szAllNotesOff[2];
sprintf(&szAllNotesOff[0], "%c", MIDI_CTL_ALL_SOUNDS_OFF);
szAllNotesOff[1] = 0x00;

// velocity 95
char szVelTarget[2];
sprintf(&szVelTarget[0], "%c", 0x5f);
szVelTarget[1] = 0x00;

char szVelTargetOff[2];
sprintf(&szVelTargetOff[0], "%c", 0x50);
szVelTargetOff[1] = 0x00;

// hihat constants
char szOpenHat[2];
sprintf(&szOpenHat[0], "%c", 0x43);
szOpenHat[1] = 0x00;

char szClosedHat[2];
sprintf(&szClosedHat[0], "%c", 0x3e);
szClosedHat[1] = 0x00;

// 0xFF eof marker for midi
char szEOFMarker[2];
sprintf(&szEOFMarker[0], "%c", 0xff);
szEOFMarker[1] = 0x00;

// null target
char szNull[2];
sprintf(&szNull[0], "%c", 0x00);
szNull[1] = 0x00;

-------------------

If any explanation or further code is needed, I'd be happy to provide.

Thanks

I don't want to sound patronizing, but it seems like you have taken on
a project way beyond your experience level. You need a better grasp
of C style strings, stdio binary file handling, and how array names
are converted to pointers to their first element in most expressions.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,755
Messages
2,569,536
Members
45,011
Latest member
AjaUqq1950

Latest Threads

Top