[Libvir] [PATCH] Enhance virBuffer code

[Jim Meyering]

"Richard W.M. Jones" <rjones at redhat.com> wrote:

> Jim Meyering wrote:
>> "Richard W.M. Jones" <rjones at redhat.com> wrote:
>>> Jim Meyering wrote:
>>>> What do you think of using this?
>>>>
>>>>   isascii (*p) && isalnum (*p)
>>> I'm not sure I'm qualified to say what this does on EBCDIC, but quite
>>> likely lots of other code breaks there too anyway.  This is nicely
>>> self-documenting anyway.
>>
>> As Daniel suggested, isalnum is locale-sensitive.
>> If there's a locale with an alphabetic byte that is outside
>> the logical a-zA-Z range, yet still within 0..127, then the above
>> expression will give a false-positive for that byte.
>>
>> I've been inclined to stop worrying about EBCDIC for years, but a quick
>> search on the web finds that people are still stuck using it, and do
>> report bugs in ASCII-assuming code.
>>
>> This is why autoconf goes to the trouble of doing this:
>>   tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ
>> not this:
>>   tr a-z A-Z
>> to convert to upper case.
>
> Another factor to consider here is that it doesn't matter if this
> function over-escapes, but it does matter if the function
> under-escapes. That is to say, it could escape every character as a
> %xx hex code, which would be ugly and slightly inefficient but not
> wrong.

IMHO, if you don't use the all-enumerating switch-based code that Daniel
objects to, it'd be good to document (in both loops) that the test is
inaccurate with EBCDIC, and explain why it's ok to get false positives.

Without comments, people might be tempted to use a similar test in a
context where the differences matter.