bug-coreutils
[Top][All Lists]
Advanced

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

Re: sort "b" option in pos2 has strange effect


From: Pádraig Brady
Subject: Re: sort "b" option in pos2 has strange effect
Date: Thu, 26 Feb 2009 13:39:23 +0000
User-agent: Thunderbird 2.0.0.6 (X11/20071008)

Jim Meyering wrote:
> Pádraig Brady wrote:
>> I've tweaked the patch a bit to simplify some code
>> and expect to push it soon.
>>
>> Thanks to my friendly LUG I confirmed that solaris 9 and 10
>> behave as expected for these commands:
>>
>> printf "a a b\nz a a\n" | sort -k2,3.0
>> printf "a  y\na z\n" | sort -k1,1b
>>
>> cheers,
>> Pádraig.
>> >From 4a1f5d98265cf74297d9e523aa99fca80cc51e3c Mon Sep 17 00:00:00 2001
>> From: =?utf-8?q?P=C3=A1draig=20Brady?= <address@hidden>
>> Date: Tue, 24 Feb 2009 08:37:18 +0000
>> Subject: [PATCH] sort: Fix two bugs with determining the end of field
>>
>> * src/sort.c: When no specific number of chars to skip
>> is specified for the end field, always skip the whole field.
>> Also never include leading spaces from next field.
>> * tests/misc/sort: Add 2 new tests for these cases.
>> * NEWS: Mention this bug fix.
>> * THANKS: Add bug reporter.
>> Reported by Davide Canova
> ...
> 
> Thanks!  Looks fine.
> Only one question:
> 
>> -  /* If we're ignoring leading blanks when computing the End
>> -     of the field, don't start counting bytes until after skipping
>> -     past any leading blanks. */
>> -  if (key->skipeblanks)
>> -    while (ptr < lim && blanks[to_uchar (*ptr)])
>> -      ++ptr;
>> +  if (echar != 0) /* We need to skip over a portion of the end field.  */
>> +    {
>> +      if (key->skipeblanks) /* blanks not counted in echar.  */
> 
> Was something wrong with the comment you're removing, above?

I thought it was too verbose. It's replaced with:
  /* blanks not counted in echar.  */
which should be obvious in along with the code?

>> +    while (ptr < lim && blanks[to_uchar (*ptr)])
>> +      ++ptr;
>>
>> -  /* Advance PTR by ECHAR (if possible), but no further than LIM.  */
>> -  remaining_bytes = lim - ptr;
>> -  if (echar < remaining_bytes)
>> -    ptr += echar;
>> -  else
>> -    ptr = lim;
>> +      ptr = MIN (lim, ptr + echar);
>> +    }

Same here. I removed the comment as the code is (now) obvious I think.

cheers,
Pádraig.




reply via email to

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