Verify that changing sys/termios.h to termios.h does not change behavior on *BSD

[clepple]

FreeBSD:

/usr/include/sys/termios.h:3:2: warning: "this file includes <sys/termios.h> which is deprecated, use <termios.h> instead" [-W#warnings]

This shouldn't be a big deal to fix, but I want to make sure that the auto-detection works as expected, and the new header doesn't cause unintended behavior.

[jimklimov]

I suppose it can be a configure-time check (e.g. clean compile without warnings), so in C sources we include one or another pathname based on the ifdef'ed flag macro.

[cschuber]

I can probably look at that.

[clepple]

@cschuber that would be awesome, thanks!

To be honest, I hadn't looked at the code for this in a while, and it turns out we already have #if defined(HAVE_SYS_TERMIOS_H). (So my earlier comment about auto-detection is not well informed.)

The litmus test is probably to simplify serial.h to just #include <termios.h> and make sure that a serial driver still works on BSD. (I am optimistic - seems like we needed speed_t which POSIX.1 says should be in termios.h).

@jimklimov minimal warnings is a reasonable goal, but I don't think we want to put the cart before the horse and say that builds need zero warnings (a la -Werror) to be successful. If sys/termios.h is deprecated, and its removal does not cause side effects, let's take that opportunity to simplify.

[cschuber]

I'll try to carve some time to look at it and base a patch on 2.7.4 (not master, as master usbuhid-ups bus faults on me since my update to head of master yesterday).

[cschuber]

The easy fix would be to check for FreeBSD in serial.h (either include sys/param.h and check for __FreeBSD_version or FreeBSD) but that only puts a band-aid over the problem. Ideally a check for termios.h should be made and if it's found do not check for sys/termios.h in configure.ac.

I'm not willing to create a separate patch for the port because once sys/termios.h is removed it will revert to termios.h. IMO, as a FreeBSD developer and maintainer of the FreeBSD nut ports, this is not a serious issue. The OP can temporarily remove sys/termios.h to verify that this is the case.

[cschuber]

That should have been __ no space FreeBSD no space __ instead of bold. Mark up languages are not my friend.

[cschuber]

slippy$ cat /usr/include/sys/termios.h 
/* $FreeBSD: head/sys/sys/termios.h 199898 2009-11-28 23:50:48Z ed $ */
#ifdef __GNUC__
#warning "this file includes <sys/termios.h> which is deprecated, use <termios.h> instead"
#endif
#include <termios.h>

slippy$ 

[clepple]

LOL, okay, I think we're good on FreeBSD.

Maybe I was confusing it with OpenBSD? https://github.com/openbsd/src/blob/master/sys/sys/termios.h

[cschuber]

Yes. It will just work when you deorbit the reference to sys/termios.h.

[jimklimov]

Thanks for the investigation! Just in case, are there any current or old BSD versions that NUT is used on, where really sys/termios.h should be used and the non-sys version does not cut it? Jim

…

On Thu, Oct 22, 2020, 05:05 Cy Schubert ***@***.***> wrote: Yes. It will just work when you deorbit the reference to sys/termios.h. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#833 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMPTFE4EAHJYU6GBTQC4GTSL6OQVANCNFSM4SSVUQNA> .

[jimklimov]

@clepple : given the reference above to how different BSD distros do differ about this header, I think I can elaborate on your comment:

minimal warnings is a reasonable goal, but I don't think we want to put the cart before the horse and say that builds need zero warnings (a la -Werror) to be successful. If sys/termios.h is deprecated, and its removal does not cause side effects, let's take that opportunity to simplify.

What I meant there is that we can extend the check in configure.ac looking for "which header among several candidates in order of preference provides the token(s) we need from it" (speed_t?) AND does so without a warning. That way we would select to include a sys/header on some systems and plain header on others.

As a side effect it would help pass CI builds that intentionally barf on warnings to help keep codebase clean and portable; sure this endeavor would take a while even for cases already configured in BuildBot and Travis CI (for the latter, setups not passing are currently not enabled by default to not waste their resources in vain). But given that many of those warnings can be practical code issues, it is beneficial to not be distracted by minor and irrelevant easy-to-fix flaws like this one, so we know that a red cross (or red ball) in a build result really needs attention :)