Re: [PATCH] sort: add --threads option to parallelize internal sort.

[Jim Meyering]

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!