5

I'm trying to write a simple program to output in hex the first 16 kilobytes of a binary file (Game Boy ROM) in 16-bit chunks. However during the for loop my program will invariably segfault, however it always segfaults at a different point in the array. Here is the code:

#include <stdio.h>
#include <stdint.h>

int main ()
{
    uint16_t buffer[8000];
    FILE* ROM = fopen("rom.gb", "rb");
    if (ROM == NULL)
    {
        printf("Error");
        fclose(ROM);
        return 1;
    }
    fread(buffer, sizeof(buffer), 1, ROM);
    int i;
    for(i = 0; i < sizeof(buffer); ++i)
    {
        if (buffer[i] < 16)
        {
            printf("000%x ", buffer[i]);
        }
        else if (buffer[i] < 256)
        {
            printf("00%x ", buffer[i]);
        }
        else if (buffer[i] < 4096)
        {
            printf("0%x ", buffer[i]);
        }
        else
        {
            printf("%x ", buffer[i]);
        }
    }
    fclose(ROM);
    return 0; 
}

Before I changed to using uint16_t instead of char (since the Game Boy has a 16-bit address space) this did not occur, and in fact if I include the declaration

unsigned char buffer2[16000]; 

next to the declaration of the first buffer I get the expected output. So my questions are, why would adding an unused variable stop the program from segfaulting? And how can I avoid having to do this and declaring a huge array which is entirely unused in the program?

Xerxes
  • 65
  • 4

2 Answers2

10

In this line:

for(i = 0; i < sizeof(buffer); ++i)

sizeof(buffer) is the size of array in bytes, if you want the number of elements use

i < (sizeof(buffer) / sizeof(buffer[0]))

David Ranieri
  • 39,972
  • 7
  • 52
  • 94
  • 2
    Thank you very much, this solved the problem and I no longer get segfaults during the loop. – Xerxes Jan 05 '15 at 11:44
1

I'm not sure you're using fread properly, if you look at the documentation, the second argument is the size of each element, whereas you're passing the size of the whole buffer.

You probably want to swap the second and third arguments, see here for usage examples.

Also note as pointed out by Alter Mann, that you need to divide the size of the buffer by the size of its type.

Community
  • 1
  • 1
Nobilis
  • 7,310
  • 1
  • 33
  • 67
  • 1
    This was also helpful, thank you. I changed that line to `fread(buffer, 2, (sizeof(buffer) / sizeof(buffer[0])), ROM);` which gives the same output but should be more correct. I'm surprised it was even working properly considering how badly I was misusing the function. – Xerxes Jan 05 '15 at 12:16