tags:

views:

273

answers:

5

I'm reading in an NES ROM file, where the first four bytes are "\x4e\x45\x53\x1a", or NES\x1a. In my actual code, the given file can be arbitrary, so I want to check to make sure this header is here. However, I'm running into some trouble, which the following code demonstrates:

#include <stdio.h>
#include <string.h>

int main()
{
 FILE *fp;
 fp = fopen("mario.nes", "rb");

 char nes[4];
 char real_nes[4] = "NES\x1a";
 fread(nes, 4, 1, fp);
 printf("A: %x\n", nes[3]);
 printf("B: %x\n", real_nes[3]);
 printf("C: %s\n", nes);
 printf("D: %s\n", real_nes);
 if (strcmp(nes, real_nes) != 0) {
     printf("not a match\n");
 }
 fclose(fp);
 return 0;
}

which returns:

A: 1a
B: 1a
C: NES?
D: NES
not a match

where the question mark is \x1a.

I'm new to C, so it's possible I'm missing something subtle (or obvious) about why the two strings don't match, and why the question mark doesn't show when printing line D, to signify that \x1a is there at the end of the string, which line B seems to indicate it should be.

+4  A: 

Some remarks and suggestions:

  • open the files in binary mode - otherwise, funny things may happen on non-POSIX systems (fixed)

    fp = fopen("mario.nes", "rb");
    
  • null-terminate your buffers if you want to print or compare them or use functions like strncmp() which accept the string's length as extra argument

    printf("C: %.4s\n", nes);
    printf("D: %.4s\n", real_nes);
    if (strncmp(nes, real_nes, 4) != 0) {
    
  • '\x1a' is the non-graphic substitute character ^Z

  • check the return values of io functions for errors
Christoph
strncmp() compares correctly, so I'll chalk it up to some weirdness due to no null termination. Thanks.
v64
Oh right, the -b flag, I forgot to use it. Thanks!
Petruza
+2  A: 

Well, one problem is your use of strcmp. This function expects a ZERO-TERMINATED string (neither nes nor *real_nes* are zero-terminated string in your code).

Another problem is fread. Use it like this:

fread(nes, 1, 4, fp); // first size_t param is size and second is member count

Change your code like this:

int main()
{
        FILE *fp;
        fp = fopen("mario.nes", "rb");

        char nes[5];
        char real_nes[5] = "NES\x1a";
        fread(nes, 1, 4, fp);
        nes[4] = '\0';
        printf("A: %x\n", nes[3]);
        printf("B: %x\n", real_nes[3]);
        printf("C: %s\n", nes);
        printf("D: %s\n", real_nes);
        if (strcmp(nes, real_nes) != 0) {
            printf("not a match\n");
        }
        fclose(fp);
        return 0;
}

And see if it works.

Pablo Santa Cruz
Pablo, why do you correct `fread(nes, 4, 1, fp)` with `fread(nes, 1, 4, fp)`, aren't them both reading 4 bytes after all? (it's a question, not a challenge)
Petruza
Yes, they're both reading 4 bytes. But you should put size of each member on the first parameter, and read count on the second one. Since he is reading chars (1 byte), first parameter should be 1. That way, if you catch fread's return value (number of elements read) you'll get the right size.
Pablo Santa Cruz
+1  A: 

The major problem in your code is:

char real_nes[4] = "NES\x1a";

This not a string, since it does not end with the nul-terminator char ('\0'). This is the same problem for 'nes'.

Just declare them like:

char real_nes[] = "NES\x1a"; /* this is a string, ended by '\0' */
char nes[sizeof real_nes];

To be sure that there is enouth place for the '\0'.

Now you can use the %s specifier, or strcmp(). Anyway, I recommand the use of strncmp() instead, like in:

if(0 != strncmp(real_nes, nes, sizeof real_nes)) { /* some stuff */ }

HTH.

Thanks for the more comprehensive explanation about strings.
v64
Even in this example, when doing `fread(nes, 4, 1, fp);` you're not adding the null terminator to nes[4] so the comparison may not work either. `nes[4] = 0;` should be added.
Petruza
+1  A: 

Do not use string functions on not-zero-terminated byte arrays.

The problem is you have two 4 byte arrays which should contain the string "NES\x1a" (no space left for '\0' since it is already 4 bytes long), but the %s format and the strcmp need a '\0' termination at the end to know the strings end. That's why it doesn't work correctly.

1.: Do not use printf with %s format on this byte array. 2.: Use memcmp to compare the bytes.

Try this instead:

int i;

printf("Read bytes: 0x");
for(i = 0; i < sizeof(nes); i ++)
  printf("%02X", nes[i]);
printf("\n");

if (memcmp(nes, real_nes, sizeof(nes)) != 0) {
  printf("not a match\n");
}
A: 

A little too late maybe, but here's how I do it:

 // Read the 16 byte iNES header
    char header[16];
    fread( header, 16, 1, file );

    // Search for the "NES^Z" signature
    if( memcmp( header, "NES\x1A", 4 ) )
    {

As Xeno proposed, with memcmp you don't care about null terminators. After all, you are not really using strings, but more like char arrays, which is not the same due to the null terminators. As you don't really need to print the signature other than for debugging, you shouldn't care using string functions at all.

Petruza