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

["Collin L. Walling" <walling@linux.vnet.ibm.com>]

On 01/25/2018 10:08 AM, Eric Blake wrote:
> 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.
>
Sound good.  I'll prepend the underscore and document the differences.
Thanks for your suggestions.

-- 
- Collin L Walling