Hello Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32029
to review the following change.
Change subject: libpayload: strtoull: Fix edge case bug with *endptr ......................................................................
libpayload: strtoull: Fix edge case bug with *endptr
strtoull() can optionally take a second pointer as an out-parameter that will be adjusted to point to the end of the parsed string. This works almost right, but misses one important edge case: when the parsed string is "0", the function will interpret the leading '0' as an octal prefix, so that the first actually parsed digit is already the terminating '\0' byte. This will cause the function to early abort, which still (correctly) returns 0 but doesn't adjust *endptr.
The early abort is pointless anyway -- the only other thing the function does is run a for-loop whose condition is the exact inverse (so it's guaranteed to run zero iterations in this case) and then adjust *endptr (which we want). So just take it out. This also technically corrects the behavior of *endptr for a completely invalid string, since the strtoull man page says
If there were no digits at all, strtoul() stores the original value of nptr in *endptr (and returns 0).
Change-Id: Idddd74e18e410a9d0b6dce9512ca0412b9e2333c Signed-off-by: Julius Werner jwerner@chromium.org --- M payloads/libpayload/libc/string.c 1 file changed, 0 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/32029/1
diff --git a/payloads/libpayload/libc/string.c b/payloads/libpayload/libc/string.c index a58efde..2266d4b 100644 --- a/payloads/libpayload/libc/string.c +++ b/payloads/libpayload/libc/string.c @@ -521,12 +521,6 @@ ptr += 2; }
- /* If the first character isn't valid, then don't - * bother */ - - if (!*ptr || !_valid(*ptr, base)) - return 0; - for( ; *ptr && _valid(*ptr, base); ptr++) ret = (ret * base) + _offset(*ptr, base);