The Boston Diaries

The ongoing saga of a programmer who doesn't live in Boston, nor does he even like Boston, but yet named his weblog/journal “The Boston Diaries.”

Go figure.

Wednesday, February 23, 2022

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 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):

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;
}

I don't care what MISRA 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:

extern void *memcpy(void *restrict s1,void const *restrict s2,size_t n);

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

extern void *memmove(void *s1, void const *s2,size_t n);

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:

if (!extract_token(tmp,sizeof(tmp),&p,(isdigit)))
{
  /* ... */
}

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.

Obligatory Picture

[The future's so bright, I gotta wear shades]

Obligatory Contact Info

Obligatory Feeds

Obligatory Links

Obligatory Miscellaneous

You have my permission to link freely to any entry here. Go ahead, I won't bite. I promise.

The dates are the permanent links to that day's entries (or entry, if there is only one entry). The titles are the permanent links to that entry only. The format for the links are simple: Start with the base link for this site: https://boston.conman.org/, then add the date you are interested in, say 2000/08/01, so that would make the final URL:

https://boston.conman.org/2000/08/01

You can also specify the entire month by leaving off the day portion. You can even select an arbitrary portion of time.

You may also note subtle shading of the links and that's intentional: the “closer” the link is (relative to the page) the “brighter” it appears. It's an experiment in using color shading to denote the distance a link is from here. If you don't notice it, don't worry; it's not all that important.

It is assumed that every brand name, slogan, corporate name, symbol, design element, et cetera mentioned in these pages is a protected and/or trademarked entity, the sole property of its owner(s), and acknowledgement of this status is implied.

Copyright © 1999-2024 by Sean Conner. All Rights Reserved.