Re: [gpsd-dev] C99 Aftermath

[Fred Wright]

On Mon, 5 Sep 2016, Gary E. Miller wrote:
> On Mon, 5 Sep 2016 16:47:45 -0700 (PDT)
> Fred Wright <address@hidden> wrote:
>
[...]
> What I would like is the C99 stuff be guarded by #ifdef __linux__.
> Then at least the Linux code can be clean.

In this context, "clean" means "compatible with some hypothetical future
compiler that demands C99 compliance with no alternative".  While such
compliance is a useful goal, it's not a terribly pressing one so far, and
certainly not one that should take precedence over keeping the code
working or fixing real bugs.

The problem with overuse of "#ifdef __linux__" is that it makes
Linux-based testing less applicable to non-Linux behavior, and thus makes
it easier for bugs to creep in for non-Linux platforms when most
developers are testing on Linux.  Keeping the code as "naturally portable"
as possible maximizes the effectiveness of testing.

To the extent that stuff *does* have to be platform specific, there's also
the question of how much of that logic should be in the code itself, vs.
how much should be handled in the "configure" setup.

> As noted above, I'd like the C99 enforced for Linux.  That will
> keep a lot of junk from creeping back in.  Can you do the #ifdef wrap
> and resubmit?

Even if one were to take that approach, there's a nontrivial amount of
work in determining the *right* thing to do in the Linux case, rather than
just fighting with a suitcase that pops out of another place as soon as
you push in one.  And to the extent that manipulating feature-test flags
is appropriate, it's probably better to do it in compiler command-line
flags or in a common include file, so that it's done consistently, rather
than doing it piecemeal in a bunch of places.  Thus, I don't think the
stuff that I took out should be there *in that form*, with or without
Linux conditionals.  In the absence of a pressing need for C99 compliance,
discretion should be the better part of valor.

The effort involved in figuring out which now-removed feature-test tweaks
*might* be appropriate hasn't actually been lost, since that remains in
the repo history even if it's not in the code.

Note also that one can test for C99 compliance at any time purely by the
command-line scons invocation, without making any code changes at all.
In theory, one could simply say "CC=c99 scons ..." in many cases, though
trying that turned up a couple of issues, one of which is a real
SConstruct bug that's easily fixed, and another that I don't know the
right solution for, but which can be avoided by using "CC='cc --std=c99'
scons ..." instead.  Thus, actually turning on "C99 mode" should be the
*last* thing done to the code, not the first, and meanwhile, anything that
can be fixed to get rid of C99 warnings and/or errors without breaking the
non-C99 case is a welcome improvement.

Even fixing the warnings that appear *without* C99 should probably be
higher priority than C99 compliance.  In fact, the whole C99 thing was
triggered by a discussion about how to get rid of a warning that still
wasn't actually fixed by any of the C99 changes. :-)

> > The changes may be a bit large for email, particularly on-list:
> > Let me know if I should post this to the list or send it privately.
>
> On list is best, the more eyes the better.

OK, I'll send the patches.  Beware of incoming mailbomb. :-)

Fred Wright