Reading a file 16 bytes at a time in C

Question

I am trying to read a file parsed from a command line argument 16 bytes at a time. I am storing the bytes in an unsigned char array. I am then trying to print the elements in their HEX format and then, if they are printable characters, I am printing them and if not I am printing a dot "." I also would like to print the offset of the byte from the beggining of the file on each new line but I want to get the rest working before I start working on that. The problem I am having is the file I am reading from is not printing so I don't think I am doing it right. I started out using fread() but I think I may be needing to use fseek() but I am unsure. If anyone could point me in the right direction or tell me if I'm doing anything wrong I'd appreciate it.

Code:

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

int main(int argc, char *argv[])
{
    FILE *fp;

    int i, j;

    unsigned char buffer[17];

    fp = fopen(argv[1], "r");

    while(!feof(fp))
    {
        while(fread(buffer, 16, 1, fp) !=0)
        {

            for(i=0; i < 16; i++)
            {
                printf("%X ", buffer[i]);
            }

            for(j = 0; j < 16; j++)
            {
                if(isprint(buffer[j]))
                    printf("%s", buffer[j]);
                else
                    printf("." );
            }

            printf("\n");
        }
    }

    fclose(fp);

    return 0;
}

Expected output:

0001:  40 4F 28 37 0B B8 FF 9D 81 5E 2E 73 B5 CC 6A 70 @O(7.....^.s..jp
0010: 0F 30 9C 6E 7E F7 53 E0 C1 5E 9A 38 C5 02 F2 33 .0.n .S..^.8...3

EDIT: Fixed problem with suggestions. Was opening file in text mode instead of binary. Changed read mode to "rb" and code is working correctly now.

Solution

You need to open the file in binary mode, using fseek/ftell on a file in text mode gives erratic values.

So open the file with

fp = fopen(argv[1], "rb");

also good practice is to always check if a function succeeded by checking the return value, in this case if (fp != NULL) ...

Instead of reading 16 byte chunks read 1 byte 16 times, otherwise if the file length is not evenly dividable by 16 you miss the last bytes

if ( fp != NULL )
{
  int read = 0;
  while((read = fread(buffer, 1, 16, fp)) > 0)
  {
    for(i=0; i < read; i++)
    {
    ...
  }
  fclose(fp);
}

IOW no need to for the outer while (feof(fp)) ...

in

printf("%s", buffer[j]);

you are not using the correct format specifier, buffer[j] is a character, not a string so write

printf("%c", buffer[j]);

EDIT: if you are not in some embedded environment with minimal stack reading 16 bytes at a time is not so effective, you can just as might read 2k or some bigger size to read faster.

Comments

[bkedge]

Ahh ok, I see where I went wrong. Fixed it an it is now printing correctly. Now I'm going to start working on the file offset. Do you know if this would be done with fseek() or fread() as well?

[AndersK]

@wh33lybrdy use fseek to position in file. you can specify an offset either from start or end depending on the second parameter e.g. SEEK_END. If you have more questions it is better to post them than continue editing this one since after a while this question becomes very confusing to read.