Re: Removing -Wunsuffixed-float-constants, -Wdouble-promotion, -Wformat-zero-length

[Eric Blake]

On 11/29/2011 02:38 PM, Jim Meyering wrote:
>> I'm not convinced about removing it from manywarnings.m4 - it's not that
>> hard to disable the warning if you don't want it, but leaving it in
>> leads to smaller executable size for the cases where 1.0F is sufficient
>> (compared to the extra size required to represent 1.0 which is 1.0D).
>> By explicitly marking F or D to all float constants, it shows you've
>> thought about which precision is worth using; omitting the suffix could
>> be the sign of sloppy code that has other problems with misuse of
>> floating point.
> 
> Inspired by that, I went to see what would be required for coreutils to pass.
> Are these "D" and "F" really worth it?
> 
> Unless there are objections (portability?)

Aargh.  I just reread C99.

F (and f) for float, and L (or l) for long double are required, but D
(or d) for double is a GNU extension.

Since we can't silence the warning without adding an explicit 'D', but
'D' is not standardized, I have changed my mind.  Let's nuke the warning.

Meanwhile, your patch for adding 'F' is okay, but not for adding 'D'.
That is,

> +++ b/lib/hash.c
> @@ -113,8 +113,8 @@ struct hash_table
>     1.0).  The growth threshold defaults to 0.8, and the growth factor
>     defaults to 1.414, meaning that the table will have doubled its size
>     every second time 80% of the buckets get used.  */
> -#define DEFAULT_GROWTH_THRESHOLD 0.8
> -#define DEFAULT_GROWTH_FACTOR 1.414
> +#define DEFAULT_GROWTH_THRESHOLD 0.8F
> +#define DEFAULT_GROWTH_FACTOR 1.414F

this change makes sense,

> @@ -238,7 +238,7 @@ hash_print_statistics (const Hash_table *table, FILE 
> *stream)
>    fprintf (stream, "# buckets:         %lu\n", (unsigned long int) 
> n_buckets);
>    fprintf (stream, "# buckets used:    %lu (%.2f%%)\n",
>             (unsigned long int) n_buckets_used,
> -           (100.0 * n_buckets_used) / n_buckets);
> +           (100.0F * n_buckets_used) / n_buckets);

but this does not (in var-args, float gets promoted to double, so you
probably aren't gaining anything by using 'float' as an intermediary,
and starting with '100.0' as double is better to begin with).

> @@ -73,7 +73,7 @@ struct timespec dtotimespec (double);
>  static inline double
>  timespectod (struct timespec a)
>  {
> -  return a.tv_sec + a.tv_nsec / 1e9;
> +  return a.tv_sec + a.tv_nsec / 1e9D;

Likewise, this one is not portable.

-- 
Eric Blake   address@hidden    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature