bug#6176: [PATCH 2/2] sort: --debug: output data independent key warnings

[Paul Eggert]

On 05/14/10 06:10, Pádraig Brady wrote:

> -    if ((1 < (key->random + key->numeric + key->general_numeric + key->month
> -              + key->version + !!key->ignore + key->human_numeric))
> +    if ((1 < (key->random + key_numeric (key) + key->month + key->version
> +              + !!key->ignore))

This change doesn't look right, since it won't catch the error of
specifying both numeric and general_numeric options.  Am I missing
something?

> sort: obsolescent key formats used.  Consider using `-k'

Something like the following diagnostic would be far more helpful for
users who are not 'sort' experts:

  sort: obsolescent key `+2 -4' used; consider `-k 3,4' instead

Can you please arrange for that?

> +static char*

Missing space before "*".

> +  /* The following is too big, but guaranteed to be "big enough". */
> +  char *opts = xstrdup (short_options);

This unnecessarily copies short_options.  Better would be:

  char *opts = xmalloc (sizeof short_options);

But, come to think of it, the interface for key_to_opts is awkward.
Callers must currently do this:

   char *opts = key_to_opts (key);
   F (opts);
   free (opts);

where F is some function.  It'd be nicer for callers to do something
like this instead:

  char opts[sizeof short_options];
  key_to_opts (key, opts);
  F (opts);

This is a bit faster and is easier to understand (at least, for me).