Bug 66909 - dehumanize_number() doesn't detect range errors between INT64_MAX and UINT64_MAX for int64_t result
Summary: dehumanize_number() doesn't detect range errors between INT64_MAX and UINT64_...
Status: RESOLVED FIXED
Alias: None
Product: libbsd
Classification: Unclassified
Component: libbsd (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Guillem Jover
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-07-14 19:02 UTC by C
Modified: 2013-07-15 01:30 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Attachments

Description C 2013-07-14 19:02:11 UTC 
==================================8<=================================

dehumanize_number(const char *buf, int64_t *num)
{
	uint64_t rval;
...
	rc = expand_number(buf, &rval);
...
	if (rval == UINT64_MAX && sign == -1) {
		errno = ERANGE;
		return -1;
	}
	*num = rval * sign;
...

==================================8<=================================

The return value is a signed 64-bit int, so any value less than -(INT64_MAX - 1) or greater than INT64_MAX is an overflow. The range checking code could look more like this:

	if (rval > INT64_MAX) {
		errno = ERANGE;
		return -1;
	} else if (rval == INT64_MAX && sign == -1) {
		errno = ERANGE;
		return -1;
	}
Comment 1 Guillem Jover 2013-07-14 23:44:22 UTC 
Hi!

Nicely spotted. Although I don't think the correction is right. I've changed the code to look something like this instead:

       uint64_t rval, rmax;
       
       rmax = INT64_MAX + 1ULL;
       if (rval > rmax || (rval == rmax && sign == 1)) {
               errno = ERANGE;
               return -1;
       }

And updated the test suite. Will be included in 0.6.1, thanks.
Comment 2 C 2013-07-15 01:30:57 UTC 
(In reply to comment #1)
> Hi!
> 
> Nicely spotted. Although I don't think the correction is right.

Yeah, I got the ends of two's complement signed integers wrong, oops! Just goes to show how easy it is to mess up.

> I've changed
> the code to look something like this instead:
> 
>        uint64_t rval, rmax;
>        
>        rmax = INT64_MAX + 1ULL;
>        if (rval > rmax || (rval == rmax && sign == 1)) {
>                errno = ERANGE;
>                return -1;
>        }
> 
> And updated the test suite. Will be included in 0.6.1, thanks.

The "&& sign == 1" in this correction wasn't intuitive for me, but after reviewing it I believe it is correct. Thanks!

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.