1

I have this code:

unsigned char *command = "0000";
unsigned char foo = (hex_char_to_int(command[0]) << 4) | hex_char_to_int(command[1]);
unsigned char bar = (hex_char_to_int(command[2]) << 4) | hex_char_to_int(command[3]);
printf("foo: %02x, bar: %02x\r\n", foo, bar);

It uses this function:

unsigned char hex_char_to_int(unsigned char ch) {
    switch (ch){
        case '0': return 0;
        case '1': return 1;
        case '2': return 2;
        case '3': return 3;
        case '4': return 4;
        case '5': return 5;
        case '6': return 6;
        case '7': return 7;
        case '8': return 8;
        case '9': return 9;
        case 'A': return 0xA;
        case 'B': return 0xB;
        case 'C': return 0xC;
        case 'D': return 0xD;
        case 'E': return 0xE;
        case 'F': return 0xF;
        case 'a': return 0xA;
        case 'b': return 0xB;
        case 'c': return 0xC;
        case 'd': return 0xD;
        case 'e': return 0xE;
        case 'f': return 0xF;
        default: return 0;
    }
}

This is the result:

"JW\xd6\x96'$$LK\x90\xbbar: 3030\r\r\n"

This is on the Keil C51 compiler, on an AT89C55WD, with printf() going over a serial port.

What is going on?

EDIT

I change the printf line to

printf("%02x%02x\r\n", (unsigned int)foo, (unsigned int)bar);

So it looks like a bug in printf. Please, programmers, never make a debugging tool that lies. I beg you.

Drew
  • 12,578
  • 11
  • 58
  • 98
  • 4
    There's no semi colon after command in your example so I'm not sure this would even compile. There's also an overflow `command[4]` is not in `command` max index is 3. – Jesus Ramos Aug 14 '13 at 20:26
  • Is this the exact sequence of code that is being used (values inside them aside)? This does not seem like it should lead to a problem. – Jesus Ramos Aug 14 '13 at 20:41
  • Also check that non formatted strings print correctly to the serial console. – Jesus Ramos Aug 14 '13 at 20:45
  • No I've sanitized the code to remove anything that could identify it. @JesusRamos yes, other strings print correctly, and other format specifies even work correctly depending on the input. – Drew Aug 14 '13 at 21:11
  • Sounds like your compiler doesn't comply with the C language standard -- C requires the compiler to promote `unsigned char` to `unsigned int` when passed as the arguments to variadic functions like `printf`. – Adam Rosenfield Aug 14 '13 at 22:09
  • Two obvious things to check. 1. Do you have stdio.h included, and get the right prototype for printf ? 2. Do you have a prototype for hex_char_to_int() before you're calling it ? – nos Aug 14 '13 at 22:30

1 Answers1

4

As far as I can tell, that code should work under any conforming C compiler.

I haven't used Keil C51, but I've seen some indications that it doesn't entirely follow the requirements of the C standard, for example in promoting narrow types.

(This answer previously included a number of possible suggestions, most of which didn't pan out. If you're curious, see the edit history.)

Apparently an unsigned char argument passed to printf is not promoted to int or unsigned int, as the c standard requires.

To work around this while keeping your code reasonably portable, add casts to explicitly convert the values of foo and bar to unsigned int:

printf("foo: %02x, bar: %02x\r\n", (unsigned int)foo, (unsigned int)bar);

(The \r normally wouldn't be necessary, since \n is automatically converted to the system's line ending sequence for text streams, but perhaps Keil C51 works differently.)

Again, it should work either way, but this change might work around a bug feature of Keil 51.

UPDATE :

I just checked the online documentation for Keil C51. The documentation for printf shows some non-standard features, including b and B to specify char types, just as l specifies long types.

b and B are not necessary in standard C, since it's not possible to pass a char (or unsigned char, or signed char) argument to printf; any such argument will be promoted to int, or possibly unsigned int. I infer from this, and from the error you've run into, that Keil C51 doesn't promote narrow arguments to variadic functions, and in particular that an unsigned char argument is not promoted either to int or to unsigned int.

That explains why

printf("%02x", foo);

didn't work, and why

printf("%02x", (unsigned int)foo);

did.

This compiler targets a small 8-bit microprocessor. It makes sense that you wouldn't want to implicitly widen single-byte arguments. The authors apparently chose performance over conformance -- which is a perfectly valid decision. (It would be nice if the documentation were more explicit about this, or maybe I've missed something.)

Probably the recommended way to print unsigned char values in hex would be:

printf("foo: %02bx, bar: %02bx\r\n", foo, bar);

Note that this is specific to Keil C51, and makes your code non-portable to other platforms. But then again, code written to run on such a small system isn't likely to be portable anyway.

Casting to unsigned int, as I suggested previously, should also work, but using "%02bx" might be more efficient in time and code size, since the arguments can be passed as single bytes.

Keith Thompson
  • 254,901
  • 44
  • 429
  • 631
  • +1, as I almost never suspect a compiler bug as a feature :-) – jxh Aug 14 '13 at 20:45
  • @jxh More like warnings are not enabled. – Jesus Ramos Aug 14 '13 at 21:14
  • Unfortunately making this change only resulted in different gibberish :( – Drew Aug 14 '13 at 21:25
  • @Drew: I've added another couple of ideas. What is `sizeof (unsigned int)`? – Keith Thompson Aug 14 '13 at 22:04
  • @KeithThompson I tried changing the text in the `printf` and it works now. I'd still like to know the root cause if possible but the problem is fixed at least. `sizeof(unsigned int)` is 2 on this chip. – Drew Aug 14 '13 at 22:06
  • My guess is that it's a bug in the compiler, specifically in the way it promotes arguments to variadic functions, not in `printf`. Try these: `unsigned char uc = 'x'; printf("0x%x\n", (unsigned int)uc); printf("0x%x\n", uc);` (or replace `\n` by `\r\n` if your system requires it). – Keith Thompson Aug 14 '13 at 22:12
  • Accepted. Its definitely a pain that I can't use my usual C documentation, but at least now I know. Thank you! – Drew Aug 14 '13 at 23:34
  • Fascinating. I don't think that I've ever seen a C compiler intentionally deviate from the language standard quite like this before. But as you said, since it's targeting an 8-bit microprocessor, there could supposedly be valid performance reasons for doing so. – Adam Rosenfield Aug 15 '13 at 15:03