1

I wrote some code to apply a patch to a binary file, the patches are delimited ascii representations of hexadecimal values, like so: 00FF00:A155F3;9210BC.

My problem is that I'm miscalculating the value of a hexadecimal by exactly 256, but only between the delimiters ':' and ';' since I use the same hex2char() function regardless of position within the code, I can't figure out why it's failing in this one specific area. I figured instead of wasting too much time on it, I'll sign up for stackoverflow and get some help.

I don't want to use any external libraries, I don't want to include anything other than stdio.h and stdlib.h (I'll remove sys/stat.h, and fread() and realloc() until EOF, but that's another problem and i'm not concerned about it)

here is the code, my main loop is way near the end and I clearly mark where I get the bad output:

#include <stdio.h>
#include <stdlib.h>
#include <sys/stat.h>


//I'll populate this later
struct _pos {
    unsigned pos;
} pos;


//raise x to the power of y
unsigned power( unsigned x, unsigned y )
{
    if ( y == 0 )
    {
        return 1;
    }
    else
    {
        return x * power( x, y - 1 );
    }
}

//convert ascii hex values to integers
unsigned h2i(const char *input, int len) {
    unsigned retval = 0;
    char cur = 0;
    int loops = 0;
    while (0 < len) {
        cur = input[--len];
        //check if we are out of the range hex would be in
        //we assume capital letters A - F
        if ( cur < 0x30 || cur > 0x46 ) {
            printf("Hex out of range: %X\n", cur);
            exit(EXIT_FAILURE);
        }

        //numerical
        if ( cur < 0x3a ) {
            cur -= 0x30;
        }

        //alphabetical
        else {
            cur -= 0x37;
        }

        retval += (cur * power(16, loops));
        ++loops;
        }
    return retval;
}

//take 2 chars from the input and get a hex value from it
char hex2char(const char *input) {
    char buff[2];
    buff[0] = input[pos.pos++];
    buff[1] = input[pos.pos++];
    printf("\nBUFF: %c%c : POS: %i\n", buff[0], buff[1], pos.pos);
    return h2i(buff, 2);
}


//turn a delimiter terminated string into a number
unsigned hex2int(const char *input, char delimiter) {
    int offset = 0;
    char buff[13]; //max int length if represented by string

    //copy string into the buffer
    while (input[offset] != delimiter) {
        if (offset == 12) {
            printf("parse error: offset\n");
            exit(EXIT_FAILURE);
        }
        buff[offset] = input[offset];
        ++offset;
    }
   //printf("BUFF: %s : OFFSET: %i\n", buff, offset);
    pos.pos += offset;
    return h2i(buff, offset);

}

char *fast_cache( char *Filename, unsigned long *size ) {
    FILE *FilePointer;
    FilePointer = fopen( Filename, "r" );
    if ( FilePointer != NULL ) {
        char *ptr;
        struct stat FileStat;
        fstat( fileno( FilePointer ), &FileStat );
        *size = FileStat.st_size;
        ptr = ( char * ) malloc( *size );
        fread( ptr, 1, *size, FilePointer );
        fclose( FilePointer );
        return ptr;
        }
    else {
        return NULL;
        }
    }


int main(int argc, char **argv)
{
    // print greeting
    printf("Hello World!\n");

    unsigned long size;
    char *file = fast_cache(argv[1], &size);
    if (file == NULL) {
        printf("You must specify an input!\n");
        exit(EXIT_FAILURE);
    }

    //okay, patchfile found, opened, and cached, time to convert
    pos.pos = 0; //file position

    //this will be the output, unused right now
    FILE *out = fopen("output", "w");
    //a line in the file is formatted like this
    //0000FF:A9DF23;A7FF11
    unsigned fileoffset = 0;
    unsigned trash = 0;


    //loop through all lines in the patchfile
    while (pos.pos < size) {
        fileoffset = hex2int(file, 0x3a);
        ++pos.pos;
        printf("offset: %u CUR: %c\n", fileoffset, file[pos.pos]);
        while(file[pos.pos] != 0x3b ) {
            //check current values with file values
            //not implimented yet, i just print it out

///////////////////////////////////////////////////////////////////
///////////This is the issue right here///////////////////////////
//printf prints a near MAX_INT number hex value > 0xFFFFFF00
//and it is off by exactly 256 every time, but that would mean overflowing
            printf("%X ", hex2char(file));

        }
//////////////////////////////////////////////////////////////////
//but right here until EOF, everything is fine and works normally
//and it's the same freaking function

        printf("\nfound delimiter: %c @ %u\n", file[pos.pos], pos.pos);
        ++pos.pos;

        //second half of the line
        while(file[pos.pos] != '\n' && pos.pos < size) {
            printf("%X ", hex2char(file));
        }
        //eat the new line if there is one
        ++pos.pos;
        printf("\nLine complete\n");
    }

    return 0;
}
11
  • 2
    Do you have some particularly good reason to do the conversion yourself instead of using fscanf with the %x conversion, or reading a string and converting with strtol? Oct 9, 2012 at 7:11
  • Your hex2char function seems to be returning an 16 bit integer where it is supposed to return 8 bit character. Is it what you want?
    – mihirj
    Oct 9, 2012 at 7:15
  • @jerry I could use fscanf, but I'm attempting to create my own conversion function as a learning exercise which will come in handy when I have the need to create the same code as above, but in assembly on a CPU that doesn't have a C compiler, eg DCPU-16
    – Seth
    Oct 9, 2012 at 7:16
  • @mihirj oops, i forgot to cast it to char, unfortunately a quick recompile yields the same results.
    – Seth
    Oct 9, 2012 at 7:20
  • 1
    A minor nitpick, but could you please come up with a more descriptive title? That you have a strange issue with some code, doesn't really tell us anything about the problem you're trying to solve/tackle. Most people here have a strange issue with their code. ;)
    – Bart
    Oct 9, 2012 at 7:25

2 Answers 2

3

When using printf in C, your chars will be promoted to ints. As they are unsigned chars, you have to mask the remaining bits.

Replace:

printf("%X ", hex2char(file));

with:

printf("%X ", hex2char(file) & 0xff);

You can find a complete description of this problem here: Printing hexadecimal characters in C and here http://en.wikipedia.org/wiki/Sign_extension

If you have to implement this in assembly, I recommend replacing your pow function by a simple bit shifting operation. X << 4 == X * 16. Also pay attention that on microcontrollers or other embedded systems, you can find a 16 bit system. In this case, the size of your int can be different.

1
  • Ah, thank you very much! Oddly enough I've never run into this problem before but a quick check shows that it was my problem, i've made sure all of my values are cleaned with "& 0xff" after the function, and now I can get to the rest of my program, and the optimizing part.
    – Seth
    Oct 9, 2012 at 8:28
2

Your code to convert string to integer is ... fantastically complicated, for what it does.

It's really overkill to involve a (recursive!) function like power() when all you need is something like x <<= 4 to shift the value to the left by 4 bits.

Without going into too much detail in your code, try replacing your hex string to integer function with something like:

/* Parse hexadecimal number at s into value stored in v. Returns pointer to first
 * character that is not part of the number. Doesn't protect against overflow.
*/
const char * parse_int_hex(const char *s, unsigned long *v)
{
  *v = 0;
  for(; ; s++ )
  {
    char here = *s;
    unsigned long dv;
    if( here >= '0' && here <= '9' )
      dv = here - '0';
    else if( here >= 'a' && here <= 'f' )
      dv = 10 + here - 'a';
    else if( here >= 'A' && here <= 'F' )
      dv = 10 + here - 'A';
    else
      return s;
    *v <<= 4;
    *v += dv;
  }
  return s;
}

The above has no dependencies, which is why the character-checking is so tedious. :) This is about the size you would expect for a plain hex-to-integer conversion routine.

1
  • Thanks for optimizing the parsing function, your code looks like the c code I hope to write eventually, but as nmenezes stated, it was a sign extension problem that was giving me trouble.
    – Seth
    Oct 9, 2012 at 8:33

Not the answer you're looking for? Browse other questions tagged or ask your own question.