RFR: 8159695: Arguments::atojulong() fails to detect overflows

[Dmitry Samersoff]

Marcus,

> Actually strtoull can return 0 if it reads no value, so to properly
> detect all errors we should check errno.

if strtoull can't read value reminder is equal to s and this case is
caught at ll 602.

other cases of non-zero errno:

  unsupported base - not possible
  overflow - possible, but would be caught by check against LLONG_MAX

Despite the fact that posix.1c requires errno to be a thread-local, it's
better to avoid errno when possible.

-Dmitry

On 2016-06-27 15:42, Marcus Larsson wrote:
> Hi,
> 
> 
> On 06/27/2016 11:25 AM, Dmitry Samersoff wrote:
>> Marcus,
>>
>> Looks good for me beside some nits.
>>
>> 597: It might be better to don't check for errno but check for LLONG_MAX
>> at 602
> 
> Actually strtoull can return 0 if it reads no value, so to properly
> detect all errors we should check errno.
> 
>>
>> 593: s[0] == '0' && ((s[1] | 0x20) == 'x') might be slightly more
>> readable.
>>
>> 602  strlen call could be replaced with
>>        (*reminder != 0 && *(reminder+1) != 0)
> 
> I'm not sure I think this makes it more readable, but I can change it if
> you want. Anyone else have an opinion on this?
> 
> Thanks,
> Marcus
> 
>> -Dmitry
>>
>> On 2016-06-27 11:35, Marcus Larsson wrote:
>>> Hi,
>>>
>>> Please review the following patch to detect overflows in atojulong. The
>>> patch uses strtoull instead of sscanf, and adds a unit test for
>>> atojulong.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~mlarsson/8159695/webrev.00/
>>>
>>> Issue:
>>> https://bugs.openjdk.java.net/browse/JDK-8159695
>>>
>>> Testing:
>>> JPRT testset hotspot and included unit test through RBT.
>>>
>>> Thanks,
>>> Marcus
>>
> 


-- 
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.