Cppcheck-2.5

[Dan George]

I was able to track down a false, false positive 8.2:

extern void misra8_2_compliant( void );
extern void misra8_2_violation();

/* Cppcheck sees a violation, here, which appears to be a false positive
 * However, the definition is missing the explicit "no parameters"
 * designation.
 */
static void misra8_2_violation_indirect( void );
static void misra8_2_violation_indirect( ) { }

void misra8_2_compliant( void )
{
    misra8_2_violation_indirect();
}

/tmp/v2wGcGL/3/misra8_2.c:8:40: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]\
static void misra8_2_violation_indirect( void );

Confusing, but now I know to look elsewhere for the actual line to fix.

[Daniel Marjamäki]

ok seems the release will be made after the weekend after all.

I would like to sign this cppcheck windows installer. Does anybody know how we do that? To buy a certificate it seems that often a business is required. But I saw this option for open source projects:
https://www.ksoftware.net/code-signing-certificates/

Does anybody know if that would be a good option for us?

[Ranier Vilela]

Hi Daniel,
I think this not the best solution, but I have used self certificate to sign my windows executables.
Has worked for antivirus.
Let me now, if you can try.

[Daniel Marjamäki]

Thanks for the suggestion. That means I must distribute my certificate also right?

I have not managed to create a certificate for cppcheck and right now it seems that cppcheck-2.5 will not be signed.

[Ranier Vilela]

Em qui., 24 de jun. de 2021 às 10:16, "Daniel Marjamäki" danielmarjamaki@users.sourceforge.net escreveu:

Thanks for the suggestion. That means I must distribute my certificate
also right?

Well, I do not distribute my own certificate.
I only signed the exe.

I have not managed to create a certificate for cppcheck and right now it
seems that cppcheck-2.5 will not be signed.

Is relatively easy, create your own certificate and sign cppcheck.exe.
1. cppcheck.cfg:
[req]
distinguished_name = req_distinguished_name
prompt = no
utf8 = yes
default_keyfile = cppcheck.pem
encrypt_key = no

[req_distinguished_name]
C = BR
ST = Goiás
L = Goiânia
O = Cppcheck
OU = Cppcheck
CN = Cppcheck
emailAddress = danielmarjamaki@users.sourceforge.net
2. openssl.exe req -x509 -days 730 -newkey rsa:4096 -config cppcheck.cfg
-keyout key.pem -out cppcheck.pem
3. openssl.exe pkcs12 -export -passout pass:XXXXXX -inkey key.pem -in
cppcheck.pem -out cppcheck.pfx
4. signtool sign /f cppcheck.pfx /p XXXXXXX cppcheck.exe

regards,
Ranier Vilela

[Daniel Marjamäki]

hmm.. looks promising..

I have signed a msi. and I don't get the windows warning..

I still have a doubt if this will work. signtool said it added some key to a key store (locally?). Could somebody try the installer I attach? Can the installation be started without alerts?

[Ranier Vilela]

Em qui., 24 de jun. de 2021 às 16:58, "Daniel Marjamäki" danielmarjamaki@users.sourceforge.net escreveu:

hmm.. looks promising..

I have signed a msi. and I don't get the windows warning..

I still have a doubt if this will work. signtool said it added some key to
a key store (locally?). Could somebody try the installer I attach? Can the
installation be started without alerts?

Unfortunately not.
Windows 10 64 bits, showed the screen asking for authorization.
Unknown supplier.
Sorry.

regards,
Ranier Vilela

[Daniel Marjamäki]

I would rather like to see that https://trac.cppcheck.net/ticket/10297 is fixed somehow. I have seen a number of such false positives in the daca@home.

[Daniel Marjamäki]

are there any unfixed blockers right now? I hope we can release cppcheck-2.5 end of next week.

[Paul Fultz]

are there any unfixed blockers right now?

The unit tests fail currently on main.

[Daniel Marjamäki]

sorry everybody!!! :-(

[Robert Reif]

There are daca crashes.

[Daniel Marjamäki]

yes. it's not very good but I still think I want to release now.

[Richard Smith]

The 8.2 change just added to HEAD resolves a lot of my 8.2 messages however I still have some. Here is an update to 8_2.c with a chopped down version of a function that still triggers 8.2 along with some other FPs.

$ cppcheck --addon=misra.py 8_2.c 
Checking 8_2.c ...
8_2.c:126:6: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-2.7]
bool send_cmd(
     ^
8_2.c:127:11: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]
    const uint32_t  cmd,
          ^
8_2.c:128:11: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]
    const uint8_t * const cmd_data,
          ^
8_2.c:129:11: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]
    const uint32_t  cmd_data_size,
          ^
8_2.c:130:5: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]
    uint32_t * const seq_out
    ^
8_2.c:142:13: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-18.8]
    uint8_t cmd_workbuf[BUF_SIZE_MAX] = {0};
            ^

[Georgiy Komarov]

Thanks! Here is a fix: https://github.com/danmar/cppcheck/pull/3311 .

[Richard Smith]

Seems to fix those 8.2 errors but the 2.7 and 18.8 remain. I'm seeing other instances of those in my code base that I think are FP. I also still see some 8.2 but I need to validate and if FP then isolate them

[Georgiy Komarov]

I believe that this violation of rule 2.7 is not a false positive. We have an unused variable seq_out in the send_cmd function. I found it inconvenient to to look for which variable actually violates rule 2.7 when the warning is emitted only on the function definition. So I suggest the following improvement: https://github.com/danmar/cppcheck/pull/3314 .

The violation of rule 18.8 can be considered as false positive, since we don't have a definition of the BUF_SIZE_MAXdirective. Right now I'm not sure, how this could be fixed.

[Richard Smith]

I believe that this violation of rule 2.7 is not a false positive. We have an unused variable seq_out in the send_cmd function. I found it inconvenient to to look for which variable actually violates rule 2.7

Ah.. oops. Looks like I created that when I chopped the function down. Referencing the variable fixes it. The enhancement to the output is very helpful. Thank you.

The violation of rule 18.8 can be considered as false positive, since we don't have a definition of the BUF_SIZE_MAXdirective. Right now I'm not sure, how this could be fixed.

Its more than just not having a definition. In my codebase it is defined however the end result has many macros associated with it. I've managed to duplicate it with just 2 macros. New 8_2.c attached with the 18.8 even when defined.

I looked at all the remaining 8.2 message in our codebase and they all appear to be real violations. However, Sometimes the out message is misleading. Here's the violation case

bool get_framing_error(void);
bool get_framing_error()
{}

Where the prototype is stored in a header file. In some cases (but not all) I get a message like below that shows the header's version. It would be more helpful if it could show the function body case where the violation is instead of the prototype.

Note I had to modify the stuff below to keep spambot happy.

misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-8.2]
bool get_framing_error(void);
     ^

I'm manged to create a simple test case where it shows the prototype instead of the body in the output.

[Richard Smith]

Oops.. it lost my new test file.

[Georgiy Komarov]

Hmm, the 8.2 warning seems to be correct to me.
The MISRA document contains the note for rule 8.2, that says that an empty parameter list is not valid in a prototype and the void keyword should be used.

The cmd_workbuf[BUF_SIZE_MAX] = {0}; generates a warning for rule 10.4. It is also seems reasonable, because we are performing an addition between signed 5 and unsigned 1u.

[Richard Smith]

Hmm, the 8.2 warnings seem to be correct to me.
The MISRA document contains the note for rule 8.2, that says that an empty parameter list is not valid in a prototype and the void keyword should be used.

I agree it is correct and not a FP. Sorry to be confusing. What I was commenting on was that the reported output would be more helpful if it showed the location where the mismatch occurs instead of the current output. ie output like this:

bool get_framing_error()
                      ^

Instead of:

bool get_framing_error(void);
     ^

The cmd_workbuf[BUF_SIZE_MAX] = {0}; generates a warning for rule 10.4. It is also seems reasonable, because we are performing an addition between signed 5 and unsigned 1u.

Ooops. I need to pay a lot more attention to what is happening in my chopped up test cases. I see the source of confusion now. The error I thought I have been duplicating is not what I was actually reproducing. My apologies. Without the rule-text output I didn't notice my examples were a completely different message that just happened to occur on the same line of code.

The FP I've been trying to reproduce this entire time is a 9.3 message and not what I have been putting up.

Attached is a reproduction of a 9.3 message I see in my codebase that I think is FP. 9.3 explicitly allows using = { 0 } to initialize. I can re-create it using cppcheck and this file directly but if I put the same code in the test case code it doesn't trigger.

[Georgiy Komarov]

What I was commenting on was that the reported output would be more helpful if it showed the location where the mismatch occurs instead of the current output.

Yes, that sounds reasonable. Here is an improvement: https://github.com/danmar/cppcheck/pull/3320 .

Attached is a reproduction of a 9.3 message I see in my codebase that I think is FP. 9.3 explicitly allows using = { 0 } to initialize.

Thanks, that's a good point. The MISRA document says that the initializer of the form { 0 } does not violate the rule 9.3. I assume that this implies that initializers in any zero form are allowed. Here is the suggested fix: https://github.com/danmar/cppcheck/pull/3322 .

[Georgiy Komarov]

Sorry, I overlooked the case for rule 9.3. According to MISRA example suite, the { 0u } initializer in your code actually violates the rule: https://github.com/jubnzv/MISRA-Example-Suite/blob/master/R_09_03.c#L37-L43 .

[Richard Smith]

Yeah.. Hmm... I'm not sure why though. There is no explanation. Do you know?

[Georgiy Komarov]

I don't know. Using zero initializers with literals seems reasonable for me, because in general the MISRA document encourages explicit type specification.

I also found the MISRA checker in PVS Studio doesn't consider such cases as violations: https://pvs-studio.com/en/docs/warnings/v2540/ .

Maybe we should do the same? I can't think any of cases with literal zero initializers that can lead to problems in the code.