On 14/10/10 13:44, Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 14/10/10 11:06, Jim Meyering wrote:
>>> I noticed that using a field number of SIZE_MAX or larger makes --debug
>>> give an invalid diagnostic:
>>>
>>> $ :|_POSIX2_VERSION=199209 src/sort --debug +$(echo 2^64-1|bc).4 -1.2
>>> src/sort: using simple byte comparison
>>> src/sort: obsolescent key `+0 -2' used; consider `-k 1,2' instead
>>> src/sort: leading blanks are significant in key 1; consider also
>>> specifying `b'
>>
>> I'd nearly call that corner case a feature,
>> as it informs you the passed count has been wrapped.
>> I.E. the obsolete syntax has a 1-less narrower range
>> and this is informing you of that fact.
>>
>> printf "1 2\n" | src/sort -sb --debug +$((2**32-1)) -1
>> src/sort: using `en_IE.UTF-8' sorting rules
>> src/sort: obsolescent key `+0 -1' used; consider `-k 1,1' instead
>> 1 2
>> _
>>
>>
>> It does mean though, that overflows on the start field
>> for obsolete syntax do sort the data, while overflows
>> with the -k syntax do not. We could detect that for
>> the old syntax, and map overflows to SIZE_MAX-1?
>>
>> I suppose we could also add a debug warning for when
>> we do overflow, something along the lines of:
>>
>> $ src/sort -sb --debug -k$((2**32)) /dev/null
>> src/sort: 4294967296 is too large, using 4294967295
>>
>> @@ -3882,6 +3882,8 @@ parse_field_count (char const *string, size_t *val,
>> char const *msgid)
>> case LONGINT_OVERFLOW:
>> case LONGINT_OVERFLOW | LONGINT_INVALID_SUFFIX_CHAR:
>> *val = SIZE_MAX;
>> + if (debug) /* Note --debug must come before keys to diagnose this. */
>> + error (0, 0, _("%" PRIuMAX " is too large, using %zu"), n, *val);
>
> That does sound like an improvement (that would require comment changes),
> but considering it's only with an outrageously large field number
> and when using obsolescent key-specifying syntax, it's probably
> not worth the trouble. But if you want to add the above, that's
> fine, too.
>
> Note however, that would not change the mapping from a
> field number of exactly SIZE_MAX to "0" in the diagnostic.
>
I'll try and fix it up for next release
