Re: [PATCH 1/4] Use nanf("") instead of nanf(NULL)

On 08/27/2018 02:32 PM, [email protected] wrote:
> From: Keith Packard <[email protected]>
>
> Newer GCC versions require a non-NULL argument to this function for
> some reason.
>
> Signed-off-by: Keith Packard <[email protected]>
> ---
>   newlib/libc/stdio/nano-vfscanf_float.c | 2 +-
>   newlib/libc/stdio/vfscanf.c            | 2 +-
>   newlib/libc/stdio/vfwscanf.c           | 2 +-
>   newlib/libc/stdlib/strtod.c            | 4 ++--
>   newlib/libc/stdlib/wcstod.c            | 6 +++---
>   5 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/newlib/libc/stdio/nano-vfscanf_float.c b/newlib/libc/stdio/nano-vfscanf_float.c
> index a81fe7f70..5df9f227c 100644
> --- a/newlib/libc/stdio/nano-vfscanf_float.c
> +++ b/newlib/libc/stdio/nano-vfscanf_float.c
> @@ -330,7 +330,7 @@ fskip:
>   	{
>   	  flp = GET_ARG (N, *ap, float *);
>   	  if (isnan (fp))
> -	    *flp = nanf (NULL);
> +	    *flp = nanf ("");
>   	  else
>   	    *flp = fp;
>   	}
> ...
Do we want to allow this recent GCC change to force a source change, when the 
GCC change (added warning) is not a solid one?  The C standard does not require 
that the argument to nan() be non-null, although invoking general, indirect, 
rules (7.1.4) the behavior is perhaps undefined.  However, the Newlib nan() 
function ignores the argument, which means in Newlib's case the warning is 
spurious.

(The nan() definition is not totally clear, which is why I said "perhaps 
undefined" in the previous paragraph.  "If tagp does not point to an n-char 
sequence or an empty string, the call is equivalent to strtod("NAN", (char**) 
NULL)."  Does this statement include a NULL pointer--in which case the behavior 
is defined--or does it not--in which case the behavior is undefined by the 
standard, although it still is in Newlib's implementation.  A NULL pointer does 
not point to an n-char sequence or an empty string, thus it could be read this 
way, without needing to go to the general invalid argument (which include NULL 
pointer) in 7.1.4.  My personal preference in reading it is to understand that 
nan(NULL) is the same as strtod("NAN", (char**)NULL).)

Using GCC 6.2.1 for an ARM64 target, there is a real code generation difference 
between using nan("") and nan(NULL) (the latter of which gives a warning).  The 
former apparently changes the "nan("")" into "__builtin_nan("")", while the 
latter produces a function call to nan().  In other words, it appears this 
change is effectively replacing Newlib-internal nan() calls with __builtin_nan() 
calls.  I would guess that the same probably applies to any GCC version which 
emits the warning for nan(NULL) (which mine does).

I am only asking the question and pointing out a few facts related to the 
question, and don't mean to imply by asking that the change is not good, just 
that these things should be considered.  For my ARM64 case, the code is smaller 
and avoids a function call, so it is good from those points of view.

Craig