Include xlocale.h rather than locale.h for strtod_l
ClosedPublic

Authored by tcberner on Apr 23 2017, 8:06 PM.

Details

Reviewers
asemke
sgerlach
adridg
aacid
Group Reviewers
FreeBSD
Summary

strtod_l is defined in xlocale.h.

This fixes:

parser.y:287:19: error: implicit declaration of function 'strtod_l' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
             double result = strtod_l(s, &remain, locale);

on FreeBSD

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
tcberner created this revision.Apr 23 2017, 8:06 PM
pino edited reviewers, added: asemke; removed: pino.Apr 23 2017, 9:10 PM
rakuco added a subscriber: rakuco.Apr 24 2017, 9:09 AM

Note that at least glibc defines strtod_l in stdlib.h, I think it's safer to include both stdlib.h for glibc and xlocale.h for OSX and the BSDs.

asemke edited edge metadata.Apr 24 2017, 9:58 AM

We had some problems here on OSX... Adding Stefan to review this change, too. Stefan, can you please verify the OSX build with this?

sgerlach edited edge metadata.Apr 28 2017, 5:42 PM

strtod_l needs stdlib.h and xlocale.h on BSD. But we also need locale.h for newlocale().

tcberner updated this revision to Diff 13943.Apr 28 2017, 7:06 PM

add stdlib.h and xlocale.h rather than replace locale.h

adridg accepted this revision.May 5 2017, 12:25 PM
adridg added a subscriber: adridg.

LGTM on FreeBSD 10.3, both gcc and clang.

This revision is now accepted and ready to land.May 5 2017, 12:25 PM
aacid requested changes to this revision.May 27 2017, 4:46 PM
aacid added a subscriber: aacid.

No repo?

This revision now requires changes to proceed.May 27 2017, 4:46 PM
In D5555#112148, @aacid wrote:

No repo?

I'm sorry, how can I set one?

tcberner set the repository for this revision to R262 LabPlot.May 28 2017, 5:24 AM
aacid added a comment.May 28 2017, 8:48 PM

If one has to trust http://illumos.org/man/3head/xlocale.h it says

As none of the definitions supplied in this header, nor this header
itself, are specified in any standard, their use should be avoided in
portable applications.

So are we really sure we want it?

https://www.freebsd.org/cgi/man.cgi?query=xlocale&sektion=3 :

CONVENIENCE FUNCTIONS
     The xlocale API includes a	number of _l suffixed convenience functions.
     These are variants	of standard C functions	that have been modified	to
     take an explicit locale_t parameter as the	final argument or, in the case
     of	variadic functions, as an additional argument directly before the for-
     mat string.  Each of these	functions accepts either NULL or
     LC_GLOBAL_LOCALE.	In these functions, NULL refers	to the C locale,
     rather than the thread's current locale.  If you wish to use the thread's
     current locale, then use the unsuffixed version of	the function.

     These functions are exposed by including <xlocale.h> after	including the
     relevant headers for the standard variant.	 For example, the strtol_l(3)
     function is exposed by including <xlocale.h> after	<stdlib.h>, which
     defines strtol(3).
aacid resigned from this revision.May 29 2017, 8:52 PM

Ok, you'll fix it if you break it ;)

This revision is now accepted and ready to land.May 29 2017, 8:52 PM