coreutils
[Top][All Lists]
Advanced

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

Re: [coreutils] [PATCH] sort: tune and refactor --debug code, and fix mi


From: Pádraig Brady
Subject: Re: [coreutils] [PATCH] sort: tune and refactor --debug code, and fix minor underlining bug
Date: Thu, 12 Aug 2010 00:06:36 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

On 05/08/10 21:29, Paul Eggert wrote:
> Formerly, the 'compare' function and some of its subroutines had a
> debugging flag, which caused them to output underlines.  This
> change refactors the code so that debugging output is
> more-separated from the actual sorting.  In the process, the
> change fixes a minor error in the debugging output.  The change
> shortens the source code and executable size a tad, and improves
> CPU performance by 2.4% on my platform with a simple benchmark (C
> locale, line sorting, no debug).

The disadvantage of separating the debugging code from the
actual sorting code is that one now has to maintain the
extent matching in 2 places, which means we're less sure that
the debug output matches what's actually being done.

Also doing within compare(), would have allowed to
show extents actually used in the comparison of arbitrary lines,
which could have been enabled with --debug=verbose in future,
or temporarily to aid development.

I also thought about outputting the total number of comparisons
it debug mode (though that should be easy to add in in any case I think).

> -  /* Disable this combination so that users are less likely
> -     to inadvertantly update a file with debugging enabled.
> -     Also it simplifies the code for handling temp files.  */
> -  if (debug && outfile)
> -    error (SORT_FAILURE, 0, _("options -o and --debug are incompatible"));
> -
>    if (debug)
>      {
> +      if (checkonly || outfile)
> +        {
> +          static char opts[] = "X --debug";
> +          opts[0] = (checkonly ? checkonly : 'o');
> +          incompatible_options (opts);
> +        }
> +

I wouldn't have removed the comment, as Eric was wondering
why -o and --debug were incompatible.
Also I'm wondering now why --check and --debug are incompatible?

cheers,
Pádraig.



reply via email to

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