[PATCH] Fix hpux10 string to real conversion defficiences

[FX Coudert]

> 2008-04-10  John David Anglin  <dave.anglin@nrc-cnrc.gc.ca>
>
>         PR fortran/35667
> 	* acinclude.m4 (LIBGFOR_CHECK_FOR_BROKEN_STRTOF,
> 	LIBGFOR_CHECK_FOR_BROKEN_STRTOD,  
> LIBGFOR_CHECK_FOR_BROKEN_STRTOLD): New
> 	check macros.
> 	* configure.ac: Use new check macros.
> 	* configure: Rebuilt.
> 	* config.h.in: Rebuilt.
> 	* io/read.c (nan_p, inf_p, gfc_strtof, gfc_strtod, gfc_strtold): New
> 	functions.
> 	(convert_real): Use new functions.

In addition to Ralf's comment (and I think the package prefix to add  
to the variables should be "libgfor"), I have the following remarks  
and questions.

Thanks for this great patch,
FX




> +static int nan_p (const char *) __attribute__((unused));
> +static int inf_p (const char *) __attribute__((unused));
> +#if defined(HAVE_STRTOLD)
> +static long double gfc_strtold (const char *, char **)  
> __attribute__((unused));
> +#endif

We tend to avoid forward definitions unless they are required. Can  
you move the "__attribute__((unused))" to the function itself and  
remove those?

>  /* convert_real()-- Convert a character representation of a floating
>   * point number to the machine number.  Returns nonzero if there is a
> - * range problem during conversion.  TODO: handle not-a-numbers and
> - * infinities.  */
> + * range problem during conversion.  */
> +
> +/* Return 1 if the string S matches the string "nan" irrespective  
> of case;
> +   otherwise return 0.  */

The comment about convert_real() should stay just above convert_real 
() itself.

> +static int
> +nan_p (const char *s)

~/devel/gfortran/trunk/libgfortran/io $ grep '^is' * | wc -l
        5
~/devel/gfortran/trunk/libgfortran/io $ grep '_p\>' * | wc -l
        0

I guess that speaks for itself :)  We don't have any *_p functions,  
but we have is_* functions in libgfortran. I suggest naming them  
is_nan() and is_inf().

> +{
> +  if (*s != 'n' && *s != 'N')
> +    return 0;
> +
> +  s++;
> +  if (*s != 'a' && *s != 'A')
> +    return 0;
> +
> +  s++;
> +  if (*s != 'n' && *s != 'N')
> +    return 0;
> +
> +  s++;
> +  if (*s != 0)
> +    return 0;

This is a really open question to both you and Jerry: in the case of  
"NaN " (ie a space following the NaN), do we still do the right thing  
here?

> +static float
> +gfc_strtof (const char *s, char **p)
> +{
> +#if ((defined(HAVE_STRTOF) && defined(HAVE_BROKEN_STRTOF)) \
> +     || defined(HAVE_BROKEN_STRTOD))
> +  const char *s1 = s;
> +  int plus = 1;
> +
> +  if (*s1 == '+')
> +    s1++;
> +  else if (*s == '-')
> +    {
> +      s1++;
> +      plus = 0;
> +    }
> +
> +  if (nan_p (s1))
> +    return plus ? __builtin_nanf ("") : -__builtin_nanf ("");
> +  else if (inf_p (s1))
> +    return plus ? __builtin_inff () : -__builtin_inff ();
> +#endif

If you have HAVE_STRTOF and HAVE_BROKEN_STRTOF, you end up with the  
following "return" being dead code. I suggest changing the above  
"#endif" into an "#else" (and adding another "#endif" at the end of  
the function).

> +
> +#if defined(HAVE_STRTOF)
> +  return strtof (s, p);
> +#else
> +  return (float) strtod (s, p);
> +#endif
> +}

Same thing for gfc_strtod() and gfc_strtold().

FX

-- 
François-Xavier Coudert
http://www.homepages.ucl.ac.uk/~uccafco/