Re: [gpsd-dev] Warnings from 32 bit FreeBSD

[Fred Wright]

On Thu, 13 Jun 2019, Gary E. Miller wrote:
> On Thu, 13 Jun 2019 13:29:52 -0700 (PDT)
> Fred Wright <address@hidden> wrote:
>
> > > > It's not in the man page because it's controlled by *other*
> > > > includes, based on *other* config flags.  The general rule is that
> > > > all double-underscore-prefixed flags are for the includes'
> > > > internal use and should not be directly set by user code.
> > >
> > > So do I need to be telepathic?  Or what?
> >
> > The "rule" is written down somewhere, though I don't remember exactly
> > where.
>
> Yeah, no one seems to recall where.  I just looked at Python.h.  They
> use the same double underscore we do.  When they figure it out, we'll
> copy.

Well, Python.h isn't necessarily the authority. :-) And using the wrong 
one can wind up fighting with the includes' internals.

> > > If anyone has the incantation to set __BSD_VISIBLE, _BSD_SOURCE, or
> > > __DARWIN_C_LEVEL a better way, then please speak up.
> >
> > Before the latest flurry of changes, I'd gotten libgpsd_core.c fixed
> > with this:
>
> Looks ptretty similar to what I just did.  Except mine get into
> gpsd_confg.h, instead of being all over the place inconsistently.
>
> > -#ifdef __linux__
> > -/* FreeBSD chokes on this */
> > -/* getsid() needs _XOPEN_SOURCE, 500 means X/Open 1995 */
> > -#define _XOPEN_SOURCE 500
> > -#endif /* __linux__ */
>
> Notice the mess here?  Fixing FreeBSD by testing for not linux?

Yeah, I know, though that particular case seems to be based on the 
particular value chosen.

> This is what I am fixing.  A FreeBSD patch should only affect FreeBSD.
>
> > +/* getsid() needs _XOPEN_SOURCE */
> > +/* 500 seems to be sufficient for Linux, but some BSDs need 700 */
> > +#define _XOPEN_SOURCE 700
>
> The code variously uses _XOPEN_SOURCE 500, 600 and 700!
>
> By pulling into gpsd_config.h there is only ever one value for _ZOPEN_SOURCE.
>
> > +#ifndef __linux__  /* Avoid deprecation warnings on some Linuces */
> >   #define _DARWIN_C_SOURCE
> > +#define _BSD_SOURCE
> > +#define _NETBSD_SOURCE
> > +#endif /* !__linux__ */
>
> Fixing for osX by again testing for not linux?  Ugh.  Gone.

Actually, that Linux fix was really for _BSD_SOURCE, but it made sense to 
put them all in the conditional.  I think the original intent was that 
each of those macros would only affect the named OS and be ignored by the 
others, so it would be safe to define several.  But then Linux decided to 
pay attention to _BSD_SOURCE for some BSD features, and then later decided 
that it really wanted to base it on _DEFAULT_SOURCE, so it may give a 
deprecation warning about the definition of _BSD_SOURCE.

> These are now added only for the proper OS.  I hope:
>
> > +#define _BSD_SOURCE
> > +#define _NETBSD_SOURCE
>
> > I'd expected something like this to work on the other files as well,
> > with the possible exception of the isascii case.  Since isascii is
> > just a trivial one-line macro, it may make more sense to provide a
> > local definition than to wrestle with the right feature-test
> > incantation to get the one-line definition from the OS includes.
>
> Ugh.  Duplicating 30 year old standard code has got to go.  Modern
> compilers inline isascii().  We can't duplicate the efficiency.

Actually, I saw it defined more often as a macro than as an inline, and in 
any case there's no efficiency benefit to the inline.  The advantages of 
the inline are:

1) Not evaluating the argument multiple times.  The optimizer would remove 
this in the simple case, anyway, but it would matter for an argument with 
side effects.  Since it's not guaranteed *not* to be a macro, supplying an 
argument with side effects should be avoided, anyway.

2) Better type checking.

[...]
> > Another possibility would be to move a few more things into the
> > os_compat module, which was intended to be the place for all the
> > messy OS_specific stuff, so that the rest of the code wouldn't need
> > any of that crud.  But again, this isn't something I'd do right
> > before a release.
>
> I looked at that.  I could not make it work.

It would mean using different names, but not something for the release.

> > > Right now osX, Linux and FreeBSD are good.  NetBSD has a few more
> > > nits to fix.
> >
> > Actually, no.  Although things have improved a bit since last night,
>
> Care to share?  Your results are not the same as from others.

Failures (mostly not including warnings):

CentOS 7:
MacCent: Linux maccent.local 3.10.0-957.5.1.el7.x86_64 #1 SMP Fri Feb 1 
14:54:57 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
MacCent: This system is: linux2
[...]
MacCent: netlib.c: In function 'netlib_localsocket':
MacCent: netlib.c:200:28: warning: implicit declaration of function 'SUN_LEN' 
[-Wimplicit-function-declaration]
MacCent:   if (connect(sock, (struct sockaddr *)&saddr, SUN_LEN(&saddr)) < 0) {
MacCent:                             ^
MacCent: os_compat.c: In function 'os_daemon':
MacCent: os_compat.c:151:5: warning: implicit declaration of function 'daemon' 
[-Wimplicit-function-declaration]
MacCent:      return daemon(nochdir, noclose);
MacCent:      ^
MacCent: driver_nmea2000.c: In function 'nmea2000_open':
MacCent: driver_nmea2000.c:1549:18: error: storage size of 'ifr' isn't known
MacCent:      struct ifreq ifr;
MacCent:                   ^
MacCent: driver_nmea2000.c:1549:18: warning: unused variable 'ifr' 
[-Wunused-variable]
MacCent: scons: *** [driver_nmea2000.o] Error 1

OpenBSD 5.6:
MacOpen: OpenBSD MacOpen.local 5.6 GENERIC.MP#333 amd64
MacOpen: This system is: openbsd5
[...]
MacOpen: cgps.c:1063: error: 'SIGWINCH' undeclared (first use in this 
function)
MacOpen: cgps.c:1063: error: (Each undeclared identifier is reported only 
once
MacOpen: cgps.c:1063: error: for each function it appears in.)
MacOpen: scons: *** [cgps.o] Error 1

Warnings (only first occurrence of each case):

Ubuntu 14.04:
MacLinux: Linux ubuntu 4.4.0-143-generic #169~14.04.2-Ubuntu SMP Wed Feb 
13 15:00:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
MacLinux: This system is: linux2
[...]
MacLinux: In file included from tests/../libgps.h:14:0,
MacLinux:                  from tests/test_libgps.c:15:
MacLinux: tests/../gpsd_config.h:9:0: warning: "_POSIX_C_SOURCE" redefined 
[enabled by default]
MacLinux:  #define _POSIX_C_SOURCE 2001112L
[...]
MacLinux: In file included from tests/../libgps.h:14:0,
MacLinux:                  from tests/test_libgps.c:15:
MacLinux: tests/../gpsd_config.h:13:0: warning: "__USE_MISC" redefined [enabled 
by default]
MacLinux:  #define __USE_MISC
[...]

Fedora 25:
MacFed: Linux MacFed.local 4.13.16-100.fc25.x86_64 #1 SMP Mon Nov 27 19:52:46 
UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
MacFed: This system is: linux2
[...]
MacFed: In file included from tests/../compiler.h:23:0,
MacFed:                  from tests/../gpsd.h:10,
MacFed:                  from tests/test_geoid.c:11:
MacFed: tests/../gpsd_config.h:9:0: warning: "_POSIX_C_SOURCE" redefined
MacFed:  #define _POSIX_C_SOURCE 2001112L
[...]
MacFed: In file included from tests/../compiler.h:23:0,
MacFed:                  from tests/../gpsd.h:10,
MacFed:                  from tests/test_geoid.c:11:
MacFed: tests/../gpsd_config.h:13:0: warning: "__USE_MISC" redefined
MacFed:  #define __USE_MISC
[...]

macOS High Sierra:
MacHS: Darwin MacHS.local 17.7.0 Darwin Kernel Version 17.7.0: Wed Feb 27 
00:43:23 PST 2019; root:xnu-4570.71.35~1/RELEASE_X86_64 x86_64
MacHS: This system is: darwin
[...]
MacHS: In file included from tests/test_geoid.c:11:
MacHS: In file included from tests/../gpsd.h:10:
MacHS: In file included from tests/../compiler.h:23:
MacHS: tests/../gpsd_config.h:15:9: warning: '__DARWIN_C_LEVEL' macro redefined 
[-Wmacro-redefined]
MacHS: #define __DARWIN_C_LEVEL _POSIX_C_SOURCE
[...]

Similar results on all macOS >=10.7

FreeBSD 10.3:
MacFree: FreeBSD MacFree 10.3-RELEASE-p20 FreeBSD 10.3-RELEASE-p20 #0: Wed Jul 
12 03:13:07 UTC 2017     address@hidden:/usr/obj/usr/src/sys/GENERIC  amd64
MacFree: This system is: freebsd10
[...]
MacFree: packet.c:1415:7: warning: implicit declaration of function 'isascii' 
is invalid in C99 [-Wimplicit-function-declaration]
MacFree:         if (!isascii(c)) {
MacFree:              ^
MacFree: packet.c: In function 'nextstate':
[...]
MacFree: gpsmon.c: In function 'main':
MacFree: gpsmon.c:1188:11: warning: implicit declaration of function 
'putenv'; did you mean 'setenv'? [-Wimplicit-function-declaration]
MacFree:      (void)putenv("TZ=UTC"); // for ctime()
MacFree:            ^~~~~~
MacFree:            setenv
MacFree: gpsd.c: In function 'main':
MacFree: gpsd.c:2052:6: warning: implicit declaration of function 'nice'; 
did you mean 'pipe'? [-Wimplicit-function-declaration]
MacFree:   if (nice(NICEVAL) == -1 && errno != 0)
MacFree:       ^~~~
MacFree:       pipe
[...]

NetBSD 6.1.5:
MacNet: NetBSD MacNet.local 6.1.5 NetBSD 6.1.5 (GENERIC) amd64
MacNet: This system is: netbsd6
[...]
MacNet: json.c: In function 'json_internal_read_object':
MacNet: json.c:405:3: warning: array subscript has type 'char'
MacNet: driver_nmea0183.c: In function 'merge_ddmmyy':
MacNet: driver_nmea0183.c:151:9: warning: array subscript has type 'char'
MacNet: driver_nmea0183.c: In function 'merge_hhmmss':
MacNet: driver_nmea0183.c:219:9: warning: array subscript has type 'char'
MacNet: driver_nmea0183.c:235:9: warning: array subscript has type 'char'
MacNet: json.c: In function 'json_internal_read_object':
MacNet: json.c:405:3: warning: array subscript has type 'char'

Fred Wright