K&R Exercise 2-3 "Hex to int converter" Problem.

Question

The program I wrote works in demographics consisting of only single Hexadecimal values. (Probably not the most elegant solution, but I'm a new programmer) My question is, how would I go about handling of multiple hexadecimal digits, such as 0xAF, or 0xFF, etc? I'm not exactly sure, and I've seemed confuse myself greatly, in the attempt. I'm not asking for someone to hold my hand, but to give me a tip where I've gone wrong in this code and thoughts on how to fix it.

Thanks :)

/* Exercise 2-3.  Write the function htoi(s), which converts a string of
 * hexadecimal digits (including an optional 0x or 0X) into it's equivalent
 * integer value. The allowable digits are 0...9 - A...F and a...f.
 * 
 */

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

#define NL '\n'
#define MAX 24

int htoi(char *hexd);

int
main(void)
{
    char str[MAX] = {0};
    char hex[] = "0123456789ABCDEFabcdef\0";
    int c;
    int i;
    int x = 0;

    while((c = getchar()) != EOF) {
        for(i = 0; hex[i] != '\0'; i++) {
            if(c == hex[i])
                str[x++] = c;
        }
        if(c == NL) {
            printf("%d\n", htoi(str));
            x = 0, i = x;
        }
    }
    return 0;
}

int
htoi(char *hexd) 
{
    int i;
    int n = 0;

    for(i = 0; isdigit(hexd[i]); i++)
        n = (16 * i) + (hexd[i] - '0');
    for(i = 0; isupper(hexd[i]); i++) /* Let's just deal with lowercase characters */
        hexd[i] = hexd[i] + 'a' - 'A';
    for(i = 0; islower(hexd[i]); i++) {
        hexd[i] = hexd[i] - 'a';
        n = (16 + i) + hexd[i] + 10;
        n = hexd[i] + 10;
    }
    return n;
}

Answer 1

Someone has alredy asked this (hex to int, k&r 2.3). Take a look, there are many good answers, but you have to fill in the blanks.

http://stackoverflow.com/questions/788026/hex-to-decimal-conversion-kr-exercise/788205#788205

Edit:

in

char hex[] = "0123456789ABCDEFabcdef\0";

The \0 is not necesary. hex is alredy nul terminated. Is len (0...f) + 1 = 17 bytes long.

Answer 2

I'll pick on one loop, and leave it to you to rethink your implementation. Specifically this:

for(i = 0; isdigit(hexd[i]); i++)
    n = (16 * i) + (hexd[i] - '0');

doesn't do what you probably think it does...

• It only processes the first span of characters where isdigit() is TRUE.
• It stops on the first character where isdigit() is FALSE.
• It doesn't run past the end because isdigit('\0') is known to be FALSE. I'm concerned that might be accidentally correct, though.
• It does correctly convert a hex number that can be expressed solely with digits 0-9.

Things to think about for the whole program:

• Generally, prefer to not modify input strings unless the modification is a valuable side effect. In your example code, you are forcing the string to lower case in-place. Modifying the input string in-place means that a user writing htoi("1234") is invoking undefined behavior. You really don't want to do that.
• Only one of the loops over digits is going to process a non-zero number of digits.
• What happens if I send 0123456789ABCDEF0123456789ABCDEF to stdin?
• What do you expect to get for 80000000? What did you get? Are you surprised?
• Personally, I wouldn't use NL for '\n'. C usage pretty much expects to see \n in a lot of contexts where the macro is not convenient, so it is better to just get used to it now...