3

I'm using a static code analyzer on a large embedded systems project at work (C/C++). Currently, all modules have several violations for:

Typedefs that indicate size and signedness should be used in place of the basic types.

However, we have a header file (footypes.h) defined that contains something along the lines of:

#ifdef LINUX_BUILD    
#include <inttypes.h>
#else
#ifdef VXWORKS_BUILD
#include <vxWorks.h>
#endif
#endif

typedef int8_t I8;
typedef uint8_t U8;
//etc

Then, actual code in a module looks like:

#include <foo/footypes.h>

void bar(U8* foo){} //Violation given here
void bar(U8 foo){} //No violation given here

As far as I can tell, this code is correct and portable- is this just a false positive, or is there something wrong with the implementation?

EDIT: I just realized that the violations are actually only given when a pointer is used- I've updated the example module code to reflect this.

Sam
  • 5,997
  • 5
  • 46
  • 66
84danie
  • 95
  • 1
  • 8
  • the root of the problem is (probably) the code analyzer tool. It might be too many levels of indirection or it might be the tool needs a configuration file that lists all the macros used in the project. Suggest reading the documentation for the tool to determine the cause of the problem. – user3629249 Apr 09 '18 at 17:04
  • See my edit! But either way I agree that I need to see the docs. The issue is this is defined as an institution specific rule, so the docs are less easily accessible. – 84danie Apr 09 '18 at 17:07
  • Looks like you have either misinterpreted the analyzer output or it has a bug. This is known as *MISRA C rule 6.3*. – Eugene Sh. Apr 09 '18 at 17:21
  • Looking further at the output, the specific violation for let's say, U16* foo, is "foo uses basic int type unsigned short instead of a typedef with size and signedness". If I'm not mistaking (I'm actually an intern and still kind of learning C), the type of foo is still U16, so this must be an error in the static analyzer – 84danie Apr 09 '18 at 17:29
  • the chain of macros for `U16` should be: `U16` >>> `uint16_t` >>> `unsigned short int` Which leads me to suspect too many levels of indirection for the tool to handle – user3629249 Apr 09 '18 at 17:37
  • 4
    Using custom types although there are standard types is bad design practice. There is no need for home-made fixed width types and the standard names as defined in `stdint.h` should be used to help future readers.. – too honest for this site Apr 09 '18 at 18:08
  • 1
    Your point is 100% true, but in this case only the latter part of your statement applies here- the standard types are being used, they just changed the names to be shorter (I agree that this is bad practice, but I can't change the entire codebase). – 84danie Apr 09 '18 at 18:25
  • @84danie I know typedefs. Anyway, as you are already at the base, it would be a good occasion to refactor them. On such old code-bases you even might find unexpected surprises about the declarations. I've had projects which defined the short names at multiple places in the same compilation unit, some with `typedef`s, others used macros. One project defined the names three times! (one `typedef`-the most recent approach, two macros, one with preceeding `undef`!). One caused trouble. – too honest for this site Apr 10 '18 at 11:23
  • Why would you not tell us what a static code analyzer tool (and version) your are using? That way it may be possible to reproduce and resolve your specific problem. – Clifford Apr 10 '18 at 13:00
  • Which of `LINUX_BUILD` or `VXWORKS_BUILD` are defined? And why? Surely only is required universally. If neither are defined where will `int8_t` etc. come from? – Clifford Apr 10 '18 at 13:09
  • @Olaf Good news is because of MISRA, undef can't be used! :D I'd love to refactor a lot of this code, unfortunately there are a lot of restrictions because it's legacy code that they don't want to change because it's proven to get the job done. However, if this issue turns out to be directly related to the overuse of typedef, then perhaps I have a case :) Thank you! – 84danie Apr 11 '18 at 16:35
  • @Clifford One of these are always defined- they're the build targets. Linux is used for the test build, vxworks is for the actual hardware build. Both headers contain the standard types (e.g. int8_t), but you can't use either/or on any system since the hardware differs so much. As far as the static code analyzer, it's semmle ODASA version 1.16. – 84danie Apr 11 '18 at 16:40
  • "it's proven to get the job done" - If I just got 1ct everytime someone stated this to me and I proved him wrong. – too honest for this site Apr 11 '18 at 16:47

1 Answers1

3

I work for Semmle and I can confirm that this is a false positive in our tool - your code looks fine to us.

The particular alert you're seeing is a custom analysis query we provide for your employer and their coding guidelines. As you discovered, that particular query has a bug such that it ignores 'acceptable' typedefs when they're used with pointer types. Thanks for bringing it to our attention - we will fix the query.

Nick Rolfe
  • 68
  • 5
  • Neat! Glad that it's a bug- even more glad that it's been brought to Semmle's attention. Thank you! :) – 84danie Apr 13 '18 at 00:17