GNU bug report logs - #6176
[PATCH 1/2] sort: add a --debug option to highlight key extents

Previous Next

Full log

Message #41 received at 6176 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> CS.UCLA.EDU>
To: Pádraig Brady <P <at> draigBrady.com>
Cc: 6176 <at> debbugs.gnu.org
Subject: Re: bug#6176: [PATCH 2/2] sort: --debug: output data independent
	key	warnings
Date: Fri, 14 May 2010 14:23:12 -0700

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.