[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Paul Eggert
Subject: bug#6176: [PATCH 2/2] sort: --debug: output data independent key warnings
Date: Fri, 14 May 2010 14:23:12 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv: Gecko/20100423 Thunderbird/3.0.4

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

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).

reply via email to

[Prev in Thread] Current Thread [Next in Thread]