Release

[Fred Wright]

On Sun, 17 Dec 2023, Hal Murray wrote:
> Fred Wright said:
>> The main issue I've found is that the "struct var" in ntp_control.c, is
>> relying on anonymous unions, which are a relatively new language feature.
>
> That is my attempt at getting a sane procedure for adding slots to the table.
> The old scheme required coordinated edits in several places and there was no
> checking that you got them right.

I'm not objecting the the scheme - just the use of the newer C feature, 
which at first glance doesn't seem to add a lot of value here.

>> Turning the "p_" and "p2_" prefixes into names of the union instances  seems
>> fairly reasonable (e.g., "p_time" becomes "p.time"), but would  require
>> changing the initializers.  I'd be willing to look into that if  I'm not
>> wasting my time.
>
> I think I just fixed that.  I'll push in a while after more local testing.

I'll try it again once you've updated it.

BTW, although names for the union types themselves aren't really needed in 
this context, it's sometimes useful to name them anyway for use by 
debuggers.  Though I believe all union type names occupy a single 
namespace, which has to be considered when choosing names.

>> There are also a bunch of warnings with some compilers, which might be  worth
>> looking at.  They're often fairly easy to fix, and sometimes indicate actual
>> problems.
>
> Which compilers?  Or rather which OS/distros?
>
> Can we set things up so that the gitlab CI stuff tells us about warnings?
>
> James suggested adding the compiler flag that turns warnings into errors.
> That won't work on the old old version of Bison that has a missing default or
> something like that.

That might be reasonable for CI testing, though it would be heavy-handed 
for the normal build procedure.  Or even having CI tests both ways might 
be useful, to distiguish between errors and warnings.  Though if CI tests 
had the ability to report error versus warning results, that would be 
unnecessary.

On Mon, 18 Dec 2023, Matthew Selsky wrote:

> On Sun, Dec 17, 2023 at 08:17:23PM -0800, Fred Wright via devel wrote:
>
> Hi Fred,
>
>> The main issue I've found is that the "struct var" in ntp_control.c, is
>> relying on anonymous unions, which are a relatively new language feature.
>> They were originally a GNU extension, eventually becoming official in C11.
>> But significantly increasing the compiler requirements just for one table
>> doesn't seem terribly desirable.
>
> Should our use of "-std=c99" have caught this?  Or is that flag not intended to catch features newer than standard X?

The latter.  The "-std=c99" option by itself only guarantees *at least* 
C99.  One needs to also include -pedantic to make it *at most* C99.

If "-std=c99" is included unconditionally, that could also be a problem, 
since compilers that default to C99 might not support the option.  Though 
if there's one of those "see if this option works" in the configure phase, 
that might take care of it.

The ruby build procedure takes the more conservative approach of 
determining whether "-std=c99" is *needed* to get C99 features, rather 
than just testing whether the option is *recognized*.  Though that test 
has a subtle bug that had me tearing my hair out for a while before 
finally noticing that the two messages that miscompared while being 
apparently identical were using different single-quote characters. :-)

>> There are also a bunch of warnings with some compilers, which might be worth
>> looking at.  They're often fairly easy to fix, and sometimes indicate actual
>> problems.
>
> Do you have specifics on distros/compilers that are showing warnings so we can run these to ground?

For a first pass, it might be easier to just fix the issues than write 
them up.

This mostly isn't about "distros".  I have a whole bunch of different 
versions of gcc and clang on my primary Mac system, so testing against 
different compilers is fairly easy (given that it honors "CC=").  I was 
able to run the build/test with 26 different compilers in about 7 minutes.
There were another 5 in the match pattern that couldn't compile 
ntp_control.c due to the anonymous unions (gcc earlier than 4.9).

For overall testing, I use a bunch of VMs with different Mac, Linux, and 
BSD installs.  Although macOS earlier than 10.13 isn't officially 
supported, I maintain patches to make it work all the way back to 10.4 
(which also happen to help OpenBSD).  I only test with the patches where 
needed.

>> I also stumbled across something (which may not be new) where it appears
>> that if libaes_siv is installed as a system library, it's preferred over the
>> bundled version.  That probably doesn't change the actual behavior, but may
>> lead to opportunistic builds.
>
> Interesting. Which distro includes libaes_siv as a system library?  We 
> don't modify libaes_siv so using the system version should be fine.

It's not a "distro".  Back when libaes_siv was first mentioned, I assumed 
that it would be an external dependency and created a port for it in 
MacPorts.  When ntpsec turned out to use its own bundled copy instead, I 
didn't add the libaes_siv port as a dependency, but it's still installed 
and maintained here.  I don't know if anyone uses that port for other 
purposes, but it's installable on any Mac running 10.4 or later.

On the Mac, the concept of "distros" is pretty meaningless, since very 
little is bundled by Apple, and what's actually installed is largely 
determined by what users have installed via MacPorts or Homebrew.

Fred Wright