CODEWITHSUNDEEP
embeddedcoding-stylestatic-analysiscoverityxc32
Kanni1303
Kanni1303

Reputation: 73

Coverity Static Analysis considers char or numbers as int in C

Both the LHS and RHS variables are uint8_t variable, but the issue is reported as " casting from int to unsigned char". I am not understanding how this can be an issue?

The same is applicable for an 8-bit numbers

All the variables listed in both issues are uint8_t

Issue 1)

CID 147563 (#2 of 2): Coding standard violation (CERT INT31-C)3. cert_violation: 
Casting (uint8_t)apX_compY_bitmask from int to unsigned char without checking its 
value may result in lost or misinterpreted data.


/* AP_X_Flash_Component_Y_Authenticated */
static uint8_t AP_component_require_auth; 

//Local variable:

uint8_t apX_compY_bitmask = 0u, port;

// other operations

AP_component_require_auth |= (uint8_t)apX_compY_bitmask;

Issue 2)

CID 148170 (#1 of 1): Coding standard violation (CERT INT31-C)5. cert_violation: 
Casting major_revision >> 3 from int to unsigned char without checking its 
value may result in lost or misinterpreted data.

Function argument:

void sb_rollb_prot_AP_FW_in_use_update(uint8_t img_idx, uint8_t port, uint8_t major_revision, bool primary_image)

//Local Variable
uint8_t x_loc, y_loc;
y_loc = major_revision >> 3;

Upvotes: 0

Views: 1464

Answers (1)

Clifford
Clifford

Reputation: 93524

To understand what has caused teh warning, you have to understand (or at least be aware) of C's somewhat arcane and sometimes surprising type promotion rules.

The C bit-wise and arithmetic operators operate on int or unsigned int or larger types, so when presented with operands of a a smaller type an implicit promotion occurs:

Consider this "experiment" for example:

#include <stdint.h>
#include <stdio.h>

int main()
{
    uint8_t a ;
    uint8_t b ;

    printf( "sizeof(a) = %zu\n", sizeof(a) ) ;
    printf( "sizeof(b) = %zu\n", sizeof(b) ) ;
    printf( "sizeof(a | b) = %zu\n", sizeof(a | b) ) ;
    printf( "sizeof((uint8_t)(a | b)) = %zu\n", sizeof((uint8_t)(a | b)) ) ;
    printf( "sizeof(a >> 3) = %zu\n", sizeof(a >> 3) ) ;
    printf( "sizeof((uint8_t)(a >> 3)) = %zu\n", sizeof((uint8_t)(a >> 3)) ) ;


    return 0;
}

The output (where int is 32-bit) is:

sizeof(a) = 1
sizeof(b) = 1
sizeof(a | b) = 4
sizeof((uint8_t)(a | b)) = 1
sizeof(a >> 3) = 4
sizeof((uint8_t)(a >> 3)) = 1

So in the first case:

AP_component_require_auth |= (uint8_t)apX_compY_bitmask;

The uint8_t cast serves no purpose since it already is that type, and certainly does not defeat the implicit conversion.

I am not familiar with CERT-C or Coverity, but in similar tools I have used, an implicit cast may be used to assert that the expression is deliberate:

AP_component_require_auth = (uint_8_t)(AP_component_require_auth | apX_compY_bitmask) ;

y_loc = (uint8_t)(major_revision >> 3) ;

As you can see it is not possible to resolve this using |= because you cannot then cast the result of the | expression before assignment.

However often it is better to maintain type agreement and avoid either implicit or explicit conversions and use int, unsigned or equal/larger sized integer type if there is no compelling reason to use a smaller type.

The issue in both cases is the assignment of an int sized type to a uint8_t. Though the first warning is somewhat confusing - probably due to the use of |= - preventing it form presenting the implicitly cast expression; you should get the same error without the unnecessary cast I think. The static analysis tool I am familiar with, would say something like:

implicit conversion to smaller type in assignment

in both these cases, which is I think is much clearer.

The Coverity warning is terse and minimal; if you go directly to the standard it is enforcing, it is much more explicit and gives rationale, examples and solutions: https://wiki.sei.cmu.edu/confluence/display/c/INT31-C.+Ensure+that+integer+conversions+do+not+result+in+lost+or+misinterpreted+data

Upvotes: 2

Related Questions