GNOME Bugzilla – Bug 747890
Revert some removed checks to make it work on FreeBSD
Last modified: 2015-04-27 01:18:50 UTC
nl_langinfo is supported by POSIX, but _NL_MEASUREMENT_MEASUREMENT isn't. We cannot assume all systems with nl_langinfo supports _NL_MEASUREMENT_MEASUREMENT. timezone is a XSI extension, but it is not available on FreeBSD, NetBSD, and OpenBSD. A function with the same name already exists on FreeBSD and NetBSD, so it is not easy to add the timezone variable. https://www.freebsd.org/cgi/man.cgi?query=timezone&apropos=0&sektion=0&manpath=FreeBSD+10.1-RELEASE&arch=default&format=html http://netbsd.gw.com/cgi-bin/man-cgi?timezone++NetBSD-current
Created attachment 301593 [details] [review] Patch to bring back needed checks
Review of attachment 301593 [details] [review]: Argh, I should have known better than relying on POSIX, I'll look into this but I don't want to reintroduce the old code. The nl_langinfo() part is especially annoying because it introduces two completely different code paths, one of which is never tested, and it uses LC_MESSAGES instead of LC_MEASUREMENT, which is wrong. Are you sure there is nothing we can use on BSDs? ::: libgweather/weather-metar.c @@ +65,3 @@ + /* mktime() assumes value is local, not UTC. Use tm_gmtoff to compensate */ +#ifdef HAVE_TM_TM_GMOFF + return tm.tm_gmtoff + mktime (&tm); This breaks glibc, because it needs _BSD_SOURCE instead of _XOPEN_SOURCE. I really do not want to readd that if there is a portable way to do it.
(In reply to Giovanni Campagna from comment #2) > Review of attachment 301593 [details] [review] [review]: > > Argh, I should have known better than relying on POSIX, I'll look into this > but I don't want to reintroduce the old code. > > The nl_langinfo() part is especially annoying because it introduces two > completely different code paths, one of which is never tested, and it uses > LC_MESSAGES instead of LC_MEASUREMENT, which is wrong. LC_MEASUREMENT isn't supported by POSIX, and it doesn't exist in *BSD. > Are you sure there is nothing we can use on BSDs? I did't find things that we can use, but it seems there are only 5 locales set to non-metric in glibc 2.21. Can we fallback to check whether the name of the locale ends with _US or _PR on non-glibc systems? > > ::: libgweather/weather-metar.c > @@ +65,3 @@ > + /* mktime() assumes value is local, not UTC. Use tm_gmtoff to > compensate */ > +#ifdef HAVE_TM_TM_GMOFF > + return tm.tm_gmtoff + mktime (&tm); > > This breaks glibc, because it needs _BSD_SOURCE instead of _XOPEN_SOURCE. > I really do not want to readd that if there is a portable way to do it. We can check the timezone variable before checking tm.tm_gmtoff, so glibc always use the timezone variable. Unfortunately, there is no autoconf macro can be used to check whether a global variable exists. I try to use the following code: AC_MSG_CHECKING([whether you have timezone global variable]) AC_COMPILE_IFELSE( [AC_LANG_PROGRAM([[#include <time.h>]], [[long offset = _Generic (timezone, long: timezone);]])], [have_timezone=yes], [have_timezone=no]) AC_MSG_RESULT([$have_timezone]) if test "$have_timezone" = "yes"; then AC_DEFINE([HAVE_TIMEZONE], [1], [Define if we have timezone global variable]) fi It works on both GNU/Linux (with GCC 4.9.2) and FreeBSD (with Clang 3.4.1), but it cannot be the real solution because there are many *BSD systems still using old compilers that don't support C11. I found timezone and tm.tm_gmtoff return values in different sign. We may need to fix it if we decide to keep both timezone and tm.tm_gmtoff.
In the end I reverted everything for 3.17.1, because it was not worth to replace some configure checks with other configure checks. Thanks for testing this early and making sure it never got in any release.