[quagga-dev 8947] Re: [PATCH] check for errno after strtoul

Ulrich Weber ulrich.weber at Sophos.com
Thu Nov 10 16:00:26 GMT 2011


Ok thanks for the hint! Attached the new version, which sets errno to zero,
always fails if errno is set and rejects negative numbers in str2tag(),
route_match_metric_compile(), VTY_GET_LONG and ospf_str2area_id().

Cheers
Ulrich

On 10.11.2011 14:27, David Lamparter wrote:
> On Thu, Nov 10, 2011 at 07:33:13AM -0500, James Carlson wrote:
>> On 11/10/11 07:16, David Lamparter wrote:
>>> On Thu, Nov 10, 2011 at 10:46:55AM +0100, Ulrich Weber wrote:
>>>> there are a couple of strtoul calls which check for ULONG_MAX
>>>> but not for errno == ERANGE.
>>>>     l = strtoul (str,&endptr, 10);
>>>>
>>>> -  if (*endptr == '\0' || l == ULONG_MAX || l>  UINT32_MAX)
>>>> +  if (*endptr == '\0' || (l == ULONG_MAX&&  errno == ERANGE) || l>  UINT32_MAX)
>>>>       return 0;
>>> When checking errno in combination with an ambiguous return value like
>>> ULONG_MAX, please clear errno before the call. Otherwise, if errno
>>> happens to be ERANGE before the call, the code will mistakenly assume
>>> that an invalid value has been entered.
>> It's not actually ambiguous, because the return value from strtoul is
>> not and has never been the correct thing to check for errors.
>>
>> For that function, if you care about errors, you *must* set errno to
>> zero before the call, and then check whether it's non-zero after.  I
>> suspect that the code above (and the change) is just wrong.
>>
> [...]
>> USAGE
>>       Because 0 and ULONG_MAX are returned on error and  are  also
>>       valid  returns  on  success, an application wishing to check
>>       for error situations  should  set  errno  to  0,  then  call
>>       strtoul(), then check errno and if it is non-zero, assume an
>>       error has occurred.
>>
>> Others are similar.  Don't change the constant; fix the code.
> Ah. Thanks for the pointer, I only saw half of the problem and didn't
> check the manpage for the correct semantics. I'll pay better attention
> next time!
>
>
> -David
> _______________________________________________
> Quagga-dev mailing list
> Quagga-dev at lists.quagga.net
> http://lists.quagga.net/mailman/listinfo/quagga-dev


-- 
Ulrich Weber | ulrich.weber at sophos.com | Senior Software Engineer
Astaro - a Sophos company | Amalienbadstr 41 | 76227 Karlsruhe | Germany
Phone +49-721-25516-0 | Fax –200 | www.astaro.com

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-strtoul-check-for-errno-ERANGE-in-case-of-ULONG_MAX_v2.patch
Type: text/x-patch
Size: 4160 bytes
Desc: not available
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20111110/986d9008/attachment-0001.bin>


More information about the Quagga-dev mailing list