[Nut-upsdev] NUT coding quality improvements

[Jim Klimov]

Hello fellow NUTs :)

  If you have been visiting the github project lately (or if you haven't -
then welcome to
https://github.com/networkupstools/nut/pulls?q=is%3Apr+sort%3Aupdated-desc+
!) there was a spree of bug hunting thanks to introduction of better
linters in modern compilers which warn about problematic coding patterns,
data type conversions, etc.

  While some of those changes impact mostly coding style, others change the
ways data might get processed (e.g. fixes to mixing of signed-integer
comparisons, various bit-width of integer types, global variables shadowed
by same-named function arguments, even formal changes like byte vs char vs
uint8_t in wire-protocl handling...) - and the fixes already applied or
still in review would benefit a lot from both review of source changes and
testing against real hardware with different connections media and
protocols. I tested some, but the park I have on hands is rather limited.

  Note that by this time most of the posted fixes passed CI and were merged
(more to come but not posted yet), so for testing against hardware you can
build the "master" branch state of the day you try it.

  Most of these are linked to issue #823
https://github.com/networkupstools/nut/issues/823 which started from a
similar smaller bug-squashing contribution and a realization that our
regular CI set-up was not very diverse with regard to tested environments
(OS distributions, CPU architectures, including bitness and endianness) as
well as to tested compiler and C/C++ versions. Currently there are dozens
of combinations where a build is "green" and those are marked as required
to pass (non-regression) for PRs to be merged. There are about as many
cases defined that are not yet "green", which are thus allowed to fail so
far, and are only executed if the Git branch name contains string
"fightwarn".

  In fact, it all started with an innocent question whether we still
support C89 / ANSI C or require a newer baseline?.. Hundreds  of commits
later we know experimentally that there is an obscene amount of work to do
for C89 compatibility, and it is likely not a warranted goal unless someone
is inclined to do the work to support current NUT on their older-standards
platform (current CI set-up would help evaluate if it is reached without
breaking anything else). These would include hundreds of cosmetic changes
like C-standard comments, but also some about missing keywords introduced
by C99 (like `__func__` used a lot in debug logging) and different C
library routines not exposed from system headers to builds with the older
standard. Some more details are in
https://github.com/networkupstools/nut/issues/915 but I did not endeavour
to list *all* the individual issues there - linked to a build console log
instead.

  More than that, it seems that no strict plain C revision works for NUT,
nor strict C with `__EXTENSIONS__` or various levels of POSIX version
macros: many system headers are not interpreted at these levels, or give
different outcomes (e.g. function signatures and data types) than for GNU C
revisions. Now, these might be interesting to fix in the name of building
NUT on systems that do not have a compiler with this dialect (GCC and CLANG
do work for us across a large range of versions, and run on every operating
system I could find including Windows - but I did not much try embedded
systems for example). I am not sure how *practical* a goal to support
strict C is, but it seems a lot more achievable if someone cares to
implement a few fixes and apply them in a lot of places, see
https://github.com/networkupstools/nut/issues/916

  Other caveats of note include:

* https://github.com/networkupstools/nut/issues/828 - Portability: MacOS X:
Undefined symbols for architecture x86_64: "___sigbits"
<https://github.com/networkupstools/nut/issues/828>: it seems that MacOS
headers for x86 define `__sigbits` as an inline function, but some of the
build flags might preclude it from being built and linked properly in
C-standard builds (seems to work for GNUC-standard cases).

* https://github.com/networkupstools/nut/issues/904 - Warnings about
possibilities of NULL pointers passed to '%s' in format strings
<https://github.com/networkupstools/nut/issues/904>: depending on C library
in use, the `printf()` family of routines may not do a sane job about `%s`
format string seeing a `NULL` vararg entry. NUT codebase is now prepared to
wrap offending arguments into ` NUT_STRARG(x)` macros, if someone hits a
system where this issue is a practical blocker. This change itself was not
pursued since there are higher-priority (wider spread) issues on the table.

* https://github.com/networkupstools/nut/issues/917 - the recent effort
also added a skeleton for building NUT CI on Windows workers (currently
with a linux-like environment provided by modern distros, and running
clang), just to see how far we can get here. Currently this last stumbled
on trying to generate the configure script, so anyone better versed in this
area is welcome to give help.

* https://github.com/networkupstools/nut/issues/869 - the future of testing
NUT on Travis CI in particular currently seems very cloudy :( We may end up
needing regular contributions or sponsorship to continue with the diverse
testing matrix set up over the recent month or two. Alternatively, people
versed with other cloud CI offers could be of great help to migrate or at
least offload some of the work currently done on Travis... for example, it
currently looks like GitHub Automation is a good way forward, maybe using
workers from free VM tiers from various cloud providers and/or dialing in
to the GHA orchestrator from community members' machines, to provide OSes
not served by GHA directly.

Jim Klimov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://alioth-lists.debian.net/pipermail/nut-upsdev/attachments/20201128/457b7e86/attachment.html>