Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34450 )
Change subject: string: implement strspn, strcspn, atol ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c File src/lib/string.c:
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c@140 PS3, Line 140: while (*str != 0) { nit: both of these loops look like they really want to be for-loops.
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c@142 PS3, Line 142: while (*p != 0) { nit: maybe better?
for (p = spn; *str != *p; p++) if (*p == '\0') return ret;
(also more analogous to the strcspn() version)
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c@147 PS3, Line 147: 0 nit: maybe that's just me, but in string contexts I prefer to write these as '\0' for clarity.
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c@159 PS3, Line 159: while (*str != 0) { nit: also for loops
https://review.coreboot.org/c/coreboot/+/34450/3/src/lib/string.c@174 PS3, Line 174: According to the man page atol() needs to be equivalent to strtol(str, NULL, 10) (except for error checking), and strtol(10) accepts an arbitrary amount of leading whitespace as well as a leading + or -.
-
Julius Werner (Code Review)