HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Hexadecimal to integer conversion function

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conversionfunctionhexadecimalinteger

Problem

I am working through Kernighan and Ritchie to brush up on my C skills, so I thought I would invite critiques of my code.

Here is my Hex-to-int function:

int htoi(char *str, unsigned long long *result)
{
    int c;
    int i;
    int msnibble;
    int digits;
    const int nibbles[] = {'\x0','\x1','\x2','\x3',     \
                '\x4','\x5','\x6','\x7',    \
                '\x8','\x9','\xA','\xB',    \
                '\xC','\xD','\xE','\xF'};   

    if (NULL == result) {
        printf("Invalid pointer\n");
        return -1;
    }

    if ('x' == str[1] || 'X' == str[1]) {
                msnibble = 2;
                digits = strlen(str) - 2;
        } else {
                msnibble = 0;
                digits = strlen(str);
        }

    if (digits > 16) {
        printf("Too many hex digits\n");
        return -1;
    }

        for (i=0, *result = 0; '\0' != (c = str[i+msnibble]); i++) {
                if ( c >= 'a' && c = 'A' && c = '0' && c <= '9') {
                        c -= '0';
                } else {
                        printf("Non Hex Character Detected; Exiting\n");
                        return -1;
                }
                *result = *result << 4 | nibbles[c];
        }
    return 0;
}


I also tried a solution using the math library. Using pow, I would get correct output until I passed 0xffffffffffffffff and then it would be one greater than the correct answer. This is why I settled on using the array of nibbles.

Edit 1:

```
int htoi(const char str, unsigned long long result)
{
int c;
int i;
int msnibble;
int digits;

if (NULL == result) {
printf("Invalid pointer\n");
return -1;
}

if ('0' == str[0]) {
if ('x' == str[1] || 'X' == str[1]) {
msnibble = 2;
digits = strlen(str) - 2;
} else {
msnibble = 0;

Solution

Your indentation is messy (non-standard and not uniform).

If the 2nd character is 'x' or'X' you don't also assert that the 1st character is '0'.

You don't signal an error if the input string has zero length.

I doubt you need \ at the end of the lines of code which initialize nibbles.

The nibbles array is a waste of time and space, because nibbles[c] has the same value as c (so you might as well write result = result << 4 | c;).

The input parameter should be const (const char*).

printf writes to stdout. If you want to log the cause of an error you should probably write to stderr instead.

Aside from that, congratulations: it works!

Context

StackExchange Code Review Q#42976, answer score: 8

Revisions (0)

No revisions yet.