[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] sort: add --threads option to parallelize internal sort.
From: |
Jim Meyering |
Subject: |
Re: [PATCH] sort: add --threads option to parallelize internal sort. |
Date: |
Thu, 11 Mar 2010 13:45:50 +0100 |
Pádraig Brady wrote:
> On 11/03/10 11:29, Chen Guo wrote:
>> How stupid of me:
>>> +int
>>
>>> +memcoll0 (const char *s1, size_t s1len, const char *s2, size_t s2len)
>>> +{
>>> + int diff;
>>> + if (!(s1len> 0&& s1[s1len] == '\0'))
>>> + abort ();
>>> + if (!(s2len> 0&& s2[s2len] == '\0'))
>>> + abort ();
>>
>> should obviously be s1[s1len - 1] and s2[s2len - 1].
>
> Right.
> I know Bruno suggestd it, but I wonder is abort() a bit drastic here?
> Perhaps something like this might be more general?
>
> if (s1len==0 && s2len==0)
> return 0
> else if (s1len==0)
> return 1;
> else if (s2len==0)
> return -1;
>
> Also s1len is a bit confusing as it's not the strlen()
> it's the size of the s1 buffer. I'd prefer s1size.
"s1len" bothered me for another reason: readability.
Please separatewordsforimprovedreadability.
i.e., s1_len or s1_size.
> I hope to do a full review soon.
Thanks!