On Sun, Mar 13, 2011 at 8:22 PM, Philip Guenther <[email protected]> wrote:
> Hmm, any test programs that are smaller and preferably not need X11?
> My little V100 takes forever with a build as is...

I don't know of any at the moment.  I'll try to look for something.

>> You should be using __sparc64__ instead of __sparc__ and _NETBSD_SOURCE
>> should be replaced with __BSD_VISIBLE.
>
> I'll second/confirm both of those.  Note that it'll need to pull in
> <sys/cdefs.h> for the latter to work.

It looks like they're relying on <machine/fenv.h> to pull in
<sys/cdefs.h>.  I can add an include in <fenv.h> too though.

> Other thoughts:
>
> The platform check in <fenv.h> should be moved inside the _FENV_H_
> inclusion guard.

Sure.

> Or maybe it can simply be removed: if the platform
> doesn't support it, then it won't have <machine/fenv.h> and the
> compilation will fail on the #include.

I thought the "not supported on this platform" error's a bit more
explanatory than failing to compile due to a missing include file.
Saves some trouble of people asking why some fenv.h-using code doesn't
compile on their ARM platform.

> Might want to wrap the function names in the declarations in <fenv.h>
> in parens, ala:
>    int    (feclearexcept)(int);
> so that possible future work to provide macros for those in
> <machine/fenv.h> don't have to go back and tweak them.

Makes sense, but do we do this in other header files?  I don't recall
having seen this elsewhere.

> These headers violate POSIX namespace rules in various ways:
>  - pulling in <sys/stdint.h>, <machine/fpu.h>, and <i386/npx.h>
> defines names that aren't permitted.
>   (seems unnecessary too; just use unsigned short/int/long long
> instead of uint16_t/uint32_t/uint64_t)
>  - the members of the fenv_t structures in the i386 and amd64 versions
> must have names in the implementation
>   namespace, ala __control, __status, __x87, etc.

I'll take a look at fixing this.

Reply via email to