Thread: Hex dump

I got this exercise and I think I've got it done. Can anyone show me how to make it better? It's a program that takes a file and makes a hex dump. The program has to print 10 bytes and the characters that correspond to the bytes. Whitespace is turned into periods. If the file has "a test file", the output should be

Code:

61 20 74 65 73 74 20 66 69 6C a.test.fil
65 00 00 00 00 00 00 00 00 00 e

This is the code I have now.

Code:

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

#define NBYTES 10 // # of bytes per row

void hex_dump(FILE *fp);
int load_bytes(FILE *fp, char row[], int size);
void print_hex(char row[], int n, int limit);
void print_row(char row[], int n);
int printable_char(int ch);

int main(void)
{
    fputs("Select a file: ", stdout);

    char filename[BUFSIZ];

    if ( fgets(filename, BUFSIZ, stdin) != NULL )
    {
        // Trim '\n' from the filename
        size_t len = strlen(filename);

        if ( filename[len - 1] == '\n' )
        {
            filename[len - 1] = '\0';
        }

        // Run the hex dump
        FILE *fp = fopen(filename, "r");

        if ( fp != NULL )
        {
            hex_dump(fp);
            fclose(fp);
        }
    }
}

// Print the contents of a file as hex
void hex_dump(FILE *fp)
{
    int n;

    do
    {
        char row[NBYTES + 1]; // +1 for '\0'

        n = load_bytes(fp, row, NBYTES);

        if ( n > 0 )
        {
            print_hex(row, n, NBYTES);
            print_row(row, n);
            putchar('\n');
        }
    }
    while ( n > 0 );
}

// Read up to size bytes into a string
// Return the number of bytes read
int load_bytes(FILE *fp, char row[], int size)
{
    int n = 0;
    int ch;

    while ( n < size && (ch = fgetc(fp)) != EOF )
    {
        row[n++] = ch;
    }

    row[n] = '\0';

    return n;
}

// Print the hex version of the characters in a string
void print_hex(char row[], int n, int limit)
{
    // Always print limit values
    // Use 0 for values past n
    for ( int i = 0; i < limit; ++i )
    {
        int value = row[i];

        if ( i >= n )
        {
            value = 0;
        }

        printf("%02X ", value);
    }
}

// Print a display version of the string
void print_row(char row[], int n)
{
    for ( int i = 0; i < n; ++i )
    {
        putchar(printable_char(row[i]));
    }
}

// Make a character printable
// even if it's whitespace
int printable_char(int ch)
{
    if ( isspace(ch) )
    {
        return '.';
    }
    else
    {
        return ch;
    }
}

Do any gurus here think it's OK or do I need to try something else?

1. make it const-correct (http://cprogramming.com/tutorial/const_correctness.html)
2. return 0;
3. inspired by 2: turn up your warning levels.

Don't flame me if I'm wrong, but aren't all variables supposed to be declared before any other statements in C?

Because fputs() is called before the declaration of the char[]

in comparison to other newbies, you're a master!

Careful, I might start getting a big head.

FWIW, one I wrote once.

That's a lot shorter than mine.

printable_char should probably accept and return char instead of int.

Why? I was trying to match the ctype.h character functions and used an int but if there's something wrong with that I'll change it.

Wouldn't "isprint" be more correct with regards to replacing unprintable chars with a dot than "isspace"?

Shouldn't '\n' be a printable character? "isprint" doesn't think so, but '\n' kept messing up the output before so I used "isspace" to turn it into '.'

# make it const-correct (http://cprogramming.com/tutorial/con...rectness.html)

Does that mean make everything that isn't supposed to change const? I remember having problems in C++ doing that once for stuff like "n" or "limit" in print_hex. I don't remember what the problems were though, just that I got compiler warnings.

return 0;

Where? I thought I was returning everything I needed to.

inspired by 2: turn up your warning levels.

This is the compile line I use.

Code:

como --timing --c99 -r -o $FileDir\$FileName.exe $File

I can't use --strict because I couldn't afford to buy the Dinkumware library for my compiler. I have to use the M$ library and it gives me errors with --strict.

Main should explicitly return 0

Why? I thought that C99 and C++ both returned 0 automatically.

Yes! print_hex for example never changes the argument row, so it should be const!

I get that part, but what about the sizes? I mean should I do this?

Code:

void print_hex(const char row[], const int n, const int limit);

Or just this?

Code:

void print_hex(const char row[], int n, int limit);

I haven't read much code that does the first one...

Dev-C++ supports C99

Not perfectly, that's why I bought Comeau C/C++ after someone told me that it supports C and C++ perfectly.

Visual Studio Express is another free compiler, though it doesn't support C99.

That's what I use as the back end, but the library doesn't like C99 so I'm still not set up completely until I get a C99 library. I can't compile C90 at all until I figure out what command line options to use that won't error on the library. I think Dinkumware will solve both those problems, but I don't have $200 to spend on it.

Should I not be using C99?