Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MISRA C compliance #8

Open
sw opened this issue Jan 30, 2017 · 0 comments
Open

MISRA C compliance #8

sw opened this issue Jan 30, 2017 · 0 comments

Comments

@sw
Copy link

sw commented Jan 30, 2017

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()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant