How to fix implicit signed to unsigned conversion, warning 373

Question

I know this has been asked a couple times, but the answers were not clear to me.

This warning seems to be relate to the type char being promoted by the compiler to unsigned char.....

I get the warning in two places in the function below at str[numbytes-i] = r + 0x30;.

What is the proper way to address this issue?

void usitoh( UINT16 val, UINT8 numbytes, char *str )       //extern
{
    UINT8 i = 0;
    UINT8 r = 0;
        
    //Convert the Base 10 Number to Base 16
    //Starting with LSB (i=1) and work to the MSB
    for (i=1; i <= numbytes; i++) 
    {
        r = val % 16;       //Get the MOD of the number
                            //The MOD of the number represents the Hex value
        val = val / 16;     //Get the Quotient of the number
        
        //Load the value into the string and 
        //add an offset to generate ASCII values
        // '0' = 0x30, 'A' = 0x41
        if (r <= 9) //Values < A
        {
            //For values 0-9 the ASCII offset is 0x30 
            //This produces and offset to get to chars 0,1,2,3,4,5,6,7,8,9
            //Warning 373 issue
            str[numbytes-i] = r + 0x30;
        }
        else        //Values > 9
        {
            //The MOD of the number represents the Hex value
            //For values 0-9 the ASCII offset is 0x41 to get to "A" and - 0x0A to account 0-9 numbers
            //This produces and offset to get to chars A,B,C,D,E,F
            //Warning 373 issue
            str[numbytes-i] = r + (0x41 - 0x0A);
        }        
    }//END of FOR Loop
}//END of "usitoh"

UPDTAE:

Thanks everyone for the suggestion and advice.

@chux I really like your “alternate code” and @Persixty the cleaner option.

Type casing as suggested fixed this issue.

A little backstory on the code.

I inherited this code as part of a project to redesign some hardware, and the client lead us to believe the code was good starting point…. Well… we took the bait, hook, line and sinker.

The code was written in MPLAB, XC8 compiler.

The hardware was quick and easy, but once we started into modifying the code we found that it as epic mess! I have never seen C-code that was so spaghetti-ed, it felt like assembly code. As we continued deeper we found several issues.

1. Over 500 warnings
2. Poor, missing or inaccurate comments
3. Poor and inconsistent coding practices.

We kept running into weird crashes and compiling issues.

After some prodding we found out that code was started by an entrepreneur, handed off to a consulting group who had several people hack at it.

So, the upper-case types are an artifact of the original author. Who had defined them in a header file of their own.

Answer 1

gcc -Wall -Wextra doesn't give me any warnings for this revised version:

#include <stdint.h>

void usitoh(uint16_t val, uint8_t numbytes, char *str ) {
    for (uint8_t i=0; i < numbytes; i++) {
        uint8_t r = val % 16;
        val /= 16;
        str[numbytes-i+1] = r + ((r <= 9) ? '0' : 'A' - 10);
    }
}

Answer 2

Could use casts. This fixes the warnings I get about narrowing int to char.

Not elegant but will quiet the warning. Better to use unsigned constants with unsigned variables.

// str[numbytes-i] = r + 0x30;
str[numbytes-i] = (char) (r + '0');
...
// str[numbytes-i] = r + (0x41 - 0x0A);
str[numbytes-i] = (char) (r + ('A' - 10u));

Alternate code:

void usitoh(unsigned val, size_t numbytes, char *str) {
  str[--numbytes] = 0;  // Include for null character termination
  while (numbytes > 0) {
    str[--numbytes] = "0123456789ABCDEF"[val % 16];
    val = val / 16;
  }
}

Answer 3

The answer is in the warning itself. Compiler is simply warning you about an implicit operation, which is known to be error prone, it may not be immediately obvious to you how this can cause bugs, but that's literally why the warning is here to remind you to think twice whether what you wrote is correct.

As an example, take an int, and assign it to a char. On you micro-controller your int is most likely 16 bits, while char is only 8 bits. If your integer value is big enough, you will lose up to half of the bits, which is bad if you didn't intend for this to happen. This is the only purpose of this warning - a warning that something you didn't intend may happen.

The correct thing to do is always to recheck if that's what you meant and then make it explicit - with a cast. This way compiler knows that you really meant to convert between two different types, possibly by narrowing from one to the other and you also checked whether the types really need to be converted here.

Answer 4

The most cautious compiler options should accept:

str[numbytes-i] = '0' + ((char)r) ;

I'd say that's the most natural expression of the intent.

"Treating the digit as char count up from the character for zero (0)".

That said I would say the clearest solution is:

str[numbytes-i] = HEX_DIGITS[r]

With static const char HEX_DIGITS[]="0123456789ABCDEF"; declared above.

It removes the if statement and even the slightest whiff of character set cleverness.