Re: [qemu-s390x] [Qemu-devel] [PATCH v4 04/10] s390-ccw: update libc

[Eric Blake]

On 01/25/2018 06:06 AM, Thomas Huth wrote:
>>> That's not "any whitespace", but only spaces.  A fully compliant
>>> implementation would be checking isspace(), but I don't expect you to
>>> implement that; at a minimum, also checking '\t' would get you closer
>>> (but not all the way to) compliance.
>>
>>
>> I'll fix the comment to be more clear.
>>
>> I think it's okay to just have the menu code treat any other kind
>> of whitespace as an error (it will check before calling atoi). I
>> added support for negatives in bothfunctions because it was easy
>> enough to do so and for the sakeof completeness.
>>
>> However, I worry trying to be 100% compliant will just bloat the
>> code when we only need it for very specific use cases.
>>
>> Would you say what we have (along with the fix to itostr below) is
>> sufficient enough?
> 
> IMHO the current way is good enough for a BIOS implementation. We're not
> doing a full replacement of glibc here ;-)

Documenting the issue is the best approach; don't bloat the code for
something that none of the callers care about.  Perhaps as simple as adding:

NOTE: This function is not quite like the standardized version in libc;
it does not handle all forms of leading space or INT_MIN.

(or whatever the actual differences are, based on your implementation
choices).

Another solution is to just not use the standardized name; keeping it
named _atoi() instead of atoi() makes it obvious that you are calling an
internal function that does not have to have standardized semantics.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature