pcg-c-basic icon indicating copy to clipboard operation
pcg-c-basic copied to clipboard

MISRA C compliance

Open sw opened this issue 8 years ago • 0 comments

Full MISRA C compliance of the C minimal library would be great, but even improving compliance is good.

Unfortunately I can't make a pull request right now, but below are some violations of the 2004 standard in pcg_basic.c. If you think this is a worthwhile endeavor, I'll try to take the time to submit a pull request.

I only have a checker for version 2004, someone else may be able to check it against the latest version (2012).

rule 2.2: only use /* ... */ comments - this would just be diligence work. I don't know if that would clash with the commenting convention in the rest of the library. Personally I would just disable that rule check, for me full compliance is not required.

rule 16.4: identical naming of parameters in declaration and definition: header file: void pcg32_srandom(uint64_t initstate, uint64_t initseq); implementation: void pcg32_srandom(uint64_t seed, uint64_t seq)

There are a few implicit conversions in pcg32_random_r that should be made explicit:

uint32_t pcg32_random_r(pcg32_random_t* rng)
{
    uint64_t oldstate = rng->state;
    rng->state = oldstate * 6364136223846793005ULL + rng->inc;

    // illegal implicit conversion from underlying MISRA type "unsigned long long" to "unsigned int" (MISRA C 2004 rule 10.1)
    uint32_t xorshifted = ((oldstate >> 18u) ^ oldstate) >> 27u;

    // illegal implicit conversion from underlying MISRA type "unsigned long long" to "unsigned int" (MISRA C 2004 rule 10.1)
    uint32_t rot = oldstate >> 59u;

    // the unary minus operator shall not be applied to an unsigned expression (MISRA C 2004 rule 12.9)
    // illegal implicit conversion from underlying MISRA type "signed char" to "unsigned int" (MISRA C 2004 rule 10.1)
    return (xorshifted >> rot) | (xorshifted << ((-rot) & 31));
}

uint32_t pcg32_boundedrand_r(pcg32_random_t* rng, uint32_t bound)
{
    // the unary minus operator shall not be applied to an unsigned expression (MISRA C 2004 rule 12.9)
    uint32_t threshold = -bound % bound;

    for (;;) {
        uint32_t r = pcg32_random_r(rng);

        // an 'if (expression)' construct shall be followed by a compound statement. The 'else' keyword shall be followed by either a compound statement, or another 'if' statement (MISRA C 2004 rule 14.9)
        if (r >= threshold)
            return r % bound;
    }
}

And a missing 'void':

// functions with no parameters shall be declared with parameter type void (MISRA C 2004 rule 16.5)
uint32_t pcg32_random()

sw avatar Jan 30 '17 09:01 sw