5

I've a function which take an uint8_t * argument :

uint8_t* ihex_decode(uint8_t *in, size_t len, uint8_t *out)
{
    uint8_t i, hn, ln;

    for (i = 0; i < len; i+=2) {
        hn = in[i] > '9' ? (in[i]|32) - 'a' + 10 : in[i] - '0';
        ln = in[i+1] > '9' ? (in[i+1]|32) - 'a' + 10 : in[i+1] - '0';

        out[i/2] = (hn << 4 ) | ln;
    }

    return out;
}

I use this function with :

uint8_t data[SPM_PAGESIZE]; // SPM_PAGESIZE = 256 bytes
uint8_t sysex_data[SPM_PAGESIZE/2];
ihex_decode(data, strlen(data), sysex_data);

But in this case, my compiler (avr-gcc) return a warning :

main.c|89|warning: pointer targets in passing argument 1 of 'strlen' differ in signedness /usr/include/string.h|399|note: expected 'const char *' but argument is of type 'uint8_t *'

So, i've found a solution by type casting the data var :

ihex_decode(data, strlen((const char *)data), sysex_data);

The warning disappears but I wonder if this solution is safe.

Is there a better way ?

Thanks

Loïc G.
  • 3,087
  • 3
  • 24
  • 36
  • 1
    Why are you using `uint8_t` for a type that is apparently `char` ? – Paul R Aug 10 '11 at 10:31
  • Because my program is running on microcontroller. So i can't have negative value. – Loïc G. Aug 10 '11 at 10:36
  • 3
    That makes no sense. The input data appears to be ASCII hex, so it's naturally char. The output data of course would remain as uint8_t. – Paul R Aug 10 '11 at 10:38
  • So you suggest me to change `uint8_t *in` to `char *in` ? – Loïc G. Aug 10 '11 at 10:46
  • Yes - that would seem to be the most appropriate thing to do. And of course the input data that you are passing via the `in` parameter should ideally be `char` too. – Paul R Aug 10 '11 at 11:07
  • 4
    The problem here isn't using uint8_t, the problem is people who believe that char is some mysterious magical type. If you check carefully, you will find that there are no negative numbers in the ASCII table. So using uint8_t for ASCII characters is perfectly fine, in fact it is the most proper type to use. The C standard happens to use char type for historical/illogical reasons, that's why you get the warning. Just typecast away such warning, uint8_t is more correct. – Lundin Aug 10 '11 at 12:47

4 Answers4

5

It is safe. The error has to do with mixing unsigned integers of 8 bits with characters, that are signed if you use just char.

I see, however, that the function accepts uint8_t and does character arithmetic, so it should accepts chars (or const chars, for the matter). Note that a character constant 'c' is of type char, and you're mixing signed and unsigned in the expressions inside ihex_decode, so you have to be careful to avoid overflows or negative numbers treated as big positive numbers.

A last style note. As in is not modified, it should read const uint8_t* in (or const char* in, as of above) in the parameters. Another style error (that may lead to very bad errors) is that you accept len as size_t, but declare the i loop variable as uint8_t. What if the string is more than 255 bytes in length?

Diego Sevilla
  • 28,636
  • 4
  • 59
  • 87
  • 2
    Char can be signed or unsigned, that is up to the implementation, AFAIK. – Rudy Velthuis Aug 10 '11 at 10:59
  • OK, @Rudy, add up "in this case" :) – Diego Sevilla Aug 10 '11 at 11:55
  • You say i'm mixing signed and unsigned inside `ihex_decode()`. When the type is signed ? In fact, if I change, in `ihex_decode()` : `'9'` to `57`, `'a'` to `97` and `'0'` to `48` (changing ASCII symbols to their decimal value), is the function can accept `uint8_t *` ? – Loïc G. Aug 10 '11 at 12:16
  • 1
    'c' is an int. In reality there is no such thing as character literals, the `' '` is just cosmetic goo to hide away the raw ASCII table for the programmer. Try to print `sizeof('c')` if you don't believe me. – Lundin Aug 10 '11 at 12:54
  • 2
    Actually, it's not safe theoretically, although no real world implementation I've ever seen would have a problem. It's possible for char to have more than 8 bits. In such an implementation, uint8_t would be have to the same size as char (since `sizeof(char)` is 1) but would ignore the top few bits. So an implementation might see a uint8_t as zero but non zero if cast to a char. Thus strlen could index off the end of the array. – JeremyP Aug 10 '11 at 14:50
  • 2
    @JeremyP: `uint8_t` is an *exact width* type, if an implementation does not have a type that is exactly 8 bits, then it does not have to provide `uint8_t`. – dreamlax Aug 27 '11 at 13:35
  • @JeremyP `uint8_t` does not have padding. If there is no type with exactly 8 bit without padding, `uint8_t` will not exist. C11 - 7.20.1.1 Exact-width integer types - 2: _The typedef name `uintN_t` designates an unsigned integer type with width N and no padding bits. ...._ and 3: _These types are optional. ..._ – 12431234123412341234123 Jan 21 '21 at 16:41
1

Everything which is non-const * can be safely casted to const * in C. It's save.

Patrick B.
  • 11,773
  • 8
  • 58
  • 101
1

This is safe. The warning (I think) pops up because you're casting from unsigned to signed.

the_nic
  • 290
  • 1
  • 4
  • I'm not entirely sure it **is** safe. What would be the purpose of the warning if there were no consequences? – dreamlax Aug 10 '11 at 10:27
  • Not because strlen() expects a **const** char * ? – Loïc G. Aug 10 '11 at 10:27
  • No, char* can always be cast to const char* implicitly, as it only strengthens the restrictions. signed <-> unsigned, however, may change the interpretation of data and break some algorithms, so there is a warning about it. 0 is the same though in both representations, so strlen should work just fine. – Mihails Strasuns Aug 10 '11 at 10:34
0

Its safe, the range of char < uint8_t.

Kamath
  • 4,461
  • 5
  • 33
  • 60