MISRA-C 2004 rule 8.1 violations

[GoogleCodeExporter]

What steps will reproduce the problem?

1. check the pb_encode.c and pb_decode.c with MISRA-C 2004 rule checker for 
violations

What is the expected output? What do you see instead?
- I expect (hope) for as few as possible violations
- I see violations of rule 8.1, where even static functions should have 
declaration.

Original issue reported on code.google.com by jarmo.to...@gmail.com on 29 Oct 2013 at 9:52

[GoogleCodeExporter]

I don't have an access to such a checker myself, and I have some doubts about 
the effectiveness of such rules in increasing software quality in such a small 
codebase. However, if it makes life easier for the users of the library, I see 
no reason not to fix these warnings.

Are there any other violations than the missing declarations for static 
functions?

Original comment by Petteri.Aimonen on 29 Oct 2013 at 12:54

• Changed state: Accepted
• Added labels: Priority-Low, Type-Enhancement
• Removed labels: Priority-Medium, Type-Defect

[GoogleCodeExporter]

Well there are reports for violations on rule 8.10, which basically states that 
"all declarations at file scope should be static where possible". This report 
mainly considers the ones flagged under "NANOPB_INTERNALS", but I guess 
changing these to static could be more difficult?

Or maybe PB_DECODERS[] could be accessed via API call when unit testing is 
required?

Original comment by jarmo.to...@gmail.com on 29 Oct 2013 at 1:11

[GoogleCodeExporter]

Yeah, I think getting rid of NANOPB_INTERNALS would be a good idea. For unit 
testing, #include "pb_decode.c" should be ok, so it is not a problem.

However, those functions were public in the beginning before I realized that 
their interfaces weren't really fit for it (issue 2). It should probably be 
safe to make them static now, they've been deprecated since nanopb-0.1.6. And 
who knows, maybe it'll help some compiler optimizations also.

Original comment by Petteri.Aimonen on 29 Oct 2013 at 1:24

[GoogleCodeExporter]

This issue was updated by revision 287207841db5.

Original comment by Petteri.Aimonen on 29 Oct 2013 at 2:35

• Changed state: FixedInGit

[GoogleCodeExporter]

These should now be fixed in the git master. If you can check with the tool for 
any further warnings, that would be good.

I didn't find an easy way to include a check in the regression tests, as 
neither splint nor GCC with full warnings seem to care about declaring static 
functions. Therefore it may be that some warnings will be reintroduced through 
carelessness in future versions..

Original comment by Petteri.Aimonen on 29 Oct 2013 at 2:38

[GoogleCodeExporter]

Fix released in nanopb-0.2.4.

Original comment by Petteri.Aimonen on 7 Nov 2013 at 2:56

• Changed state: FixReleased

[GoogleCodeExporter]

With revision 0.2.6 there are some new MISRA-C warnings (8.11):
the 'static' storage class specifier shall be used in definitions and 
declarations of objects and functions that have internal linkage (MISRA C 2004 
rule 8.11) 

Lines:
pb_decode.c 227, 238, 838,854,870, 876, 882, 898, 914
pb_encode.c 546, 564, 578, 592, 598, 604, 621, 640

Original comment by jarmo.to...@gmail.com on 28 Mar 2014 at 7:12

[GoogleCodeExporter]

Hmm yeah, forgot to make them static when the NANOPB_INTERNALS was removed.

Have to check if splint could catch atleast some of these.

Original comment by Petteri.Aimonen on 28 Mar 2014 at 7:27

• Changed state: Accepted

[GoogleCodeExporter]

This issue was updated by revision 70dee34da657.

Original comment by Petteri.Aimonen on 2 Apr 2014 at 6:08

• Changed state: FixedInGit

[GoogleCodeExporter]

Fix released in nanopb-0.2.8

Original comment by Petteri.Aimonen on 7 Apr 2014 at 5:51

• Changed state: FixReleased