* * * * *

         Of course I'm opinionated—if I wasn't, I would be in a cult

One of the other developers on my team ran part of our code through SonarQube
[1] and well … I have issues with its issues with our C code.

Out of the 600+ issues it flagged, about 500 or so seem to be related to the
use of restrict in the code. For example (using my own code):

-----[ C ]-----
int btm_cmp(
       struct btm const *restrict d1, /* it doesn't like this */
       struct btm const *restrict d2  /* or this */
)
{
 int rc;

 assert(d1 != NULL);
 assert(d2 != NULL);

 if ((rc = d1->year  - d2->year))  return rc;
 if ((rc = d1->month - d2->month)) return rc;
 if ((rc = d1->day   - d2->day))   return rc;
 if ((rc = d1->part  - d2->part))  return rc;

 return 0;
}
-----[ END OF LINE ]-----

I don't care what MISRA (Motor Industry Software Reliability Association) [2]
says about it—it signals intent. That these two pointer parameters to the
same type are distinct objects and should not be the same object! All you
have to see is the function prototypes for the standard functions memcpy()
and memmove() to see this. memcpy() is:

-----[ C ]-----
extern void *memcpy(void *restrict s1,void const *restrict s2,size_t n);
-----[ END OF LINE ]-----

and thus, the two memory regions aren't supposed to overlap; for memmove():

-----[ C ]-----
extern void *memmove(void *s1, void const *s2,size_t n);
-----[ END OF LINE ]-----

the function states the memory regions can overlap. Intent. Geeze.

Of the rest, I don't agree its arbitrary limit of 20 items in a union. The
union in question describes packet types of a custom protocol, and I will not
split it up just to satisfy some arbitrary limit in the scanning software.
Who came up with goto labels being all upper case? No, I don't agree with
that as it clashes with over fourty years of C convention where purely
uppercase is reserved for constants and macros. I don't agree with the
excessive casts it suggests, and I don't agree with removing the one cast I
do have.

I disagree about the “useless” parentheses around isdigit because I'm
signalling my itent to take the address of the function, not the macro. The
C99 standard says this (from section 7.1.4):

> Any function declared in a header may be additionally implemented as a
> function-like macro defined in the header, so if a library function is
> declared explicitly when its header is included, one of the techniques
> shown below can be used to ensure the declaration is not affected by such a
> macro. Any macro definition of a function can be suppressed locally by
> enclosing the name of the function in parentheses, because the name is then
> not followed by the left parenthesis that indicates expansion of a macro
> function name. For the same syntactic reason, it is permitted to take the
> address of a library function even if it is also defined as a macro.
>

(emphasis added). The code it's complaining about is:

-----[ C ]-----
if (!extract_token(tmp,sizeof(tmp),&p,(isdigit)))
{
 /* ... */
}
-----[ END OF LINE ]-----

The various is*() functions are often defined as macros (they are on the
compilers we use). I also dislik the “remove useless parentheses” crowd, if
only because the C precedence table is screwed up compared to most other
languages.

I'm not going to refactor the code just because some scanner thinks the
function is doing too much—it's not. Yes, the function itself might be long,
but it's converting a rather complex structure from C to Lua (or Lua back to
C). I'm not going to break it up just because. Besides, naming things is one
of the two hard problems in Computer Science (the others being cache
invalidation and off-by-one errors) and I would have to come up with some
name for all the new one-use only functions.

But I'm not going to reject everything it said. The suggestions such as
reducing scope of variables or making some const are fine. And there were two
bugs found, but overall, there was quite a bit of noise to go through.
Hopefully, I can argue my case for the ones I disagree with.

We shall see.

[1] https://www.sonarqube.org/
[2] https://www.misra.org.uk/

Email author at [email protected]

You are viewing proxied material from gopher.conman.org. The copyright of proxied material belongs to its original authors. Any comments or complaints in relation to proxied material should be directed to the original authors of the content concerned. Please see the disclaimer for more details.