gpsd-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[gpsd-dev] C99 Aftermath


From: Fred Wright
Subject: [gpsd-dev] C99 Aftermath
Date: Mon, 5 Sep 2016 16:47:45 -0700 (PDT)

The aborted attempt to make the code fully C99-compliant left it
completely broken for both FreeBSD and OpenBSD.  I spent quite a bit of
time trying to see if there was a practical way to incrementally fix it,
but that seemed to head down a rabbit hole as deep as the one it was
going into before the C99 stuff was turned back off.  So I decided that it
was better to roll back most of the C99-related changes, keeping only the
ones that made reasonably safe improvements without breaking anything.

Aside from the one real bugfix (the inappropriate "const" on the lat/lon
variables) and the actual C99 enable/disable, the C99 changes cane be
divided into three categories:

1) Deprecation fixes (uint, bzero() vs. memset(), etc.).

2) Defines of double-underscore feature-test flags.

3) Defines of single-underscore feature-test flags.

Category 1 represents fixes worth keeping.  Category 2 is something which
is expressly prohibited; only the compiler itself is supposed to set those
flags.  Category 3 is something which is potentially OK, but those flags
interact in complex and platform-dependent ways, and using them requires
careful study of the ramifications (and possible help from the "configure"
stuff if there's no one-size-fits all combination).

Thus, I took the approach of keeping #1 and getting rid of #2 and #3.  In
the process, I redid a couple of the fixes differently:

A) I created a single gps_usleep() function which is a drop-in replacement
for usleep().  This results in code which is smaller and more
maintainable.  It's also less error-prone, since one only has to get the
arithmetic right once. :-)

B) I got rid of the single use of the generally discouraged alloca() by
noting that the maximum buffer size needed was both modest and
determinable at compile time, so a fixed-length array can be used (though
a bit of tweaking was needed).

To do this I started with a revert of all the "C99 group" commits except
the ones being retained verbatim, and thoroughly tested the result (built
and passed regression on 14 targets).  I then applied partial versions of
the two commits that mixed categories 1 and 3, and then made the A and B
fixes described above.

After all this, I tested three states of the code against 14 targets:
the pre-c99 state (ed7780b), the current (as of yesterday) master state
(5808aa4), and my "c99-rollback" state.  This showed that the current
master state eliminates one warning on OSX 10.11 while creating new ones
on both CentOS 7 and NetBSD 6, and also completely breaks FreeBSD 10.1 and
OpenBSD 5.6.  Meanwhile, the "c99-rollback" state gets rid of the same one
warning without creating any new ones or breaking any builds, while
retaining some useful deprecation cleanups.

The changes may be a bit large for email, particularly on-list:

MacPro:gpsd fw$ wc -l 0*
     927 0001-Reverts-most-C99-related-changes.patch
     114 0002-bzero-is-gone-in-POSIX-2008.-Use-memset.patch
      30 0003-bcopy-gone-in-POSIX-2008.patch
     222 0004-Replaces-deprecated-usleep-with-nanosleep-based-alte.patch
     153 0005-Fixes-usleep-and-other-issues-in-contrib-programs.patch
      92 0006-Eliminates-the-one-use-of-the-portability-challenged.patch
    1538 total

Let me know if I should post this to the list or send it privately.  It
can also be fetched from the c99-rollback branch in my GitHub repo.

Diffs viewable online here:

        https://github.com/fhgwright/gpsd/compare/master...c99-rollback

This is the combined diff; click on a commit ID to see any per-commit
diff.

Fred Wright



reply via email to

[Prev in Thread] Current Thread [Next in Thread]