On 12/01/2014 04:10 AM, Pekka Paalanen wrote:
other = strtol(pid, &end, 0);
if (end != pid + 10) {
weston_log("can't parse lock file %s\n",
lockfile);
close(fd);
errno = EEXIST;
return -1;
}
'pid' is a fixed size string read in from the lock file, which is
converted into a number of type pid_t. Because the number is assumed to
be printed by "%10d\n", the file should have at least 11 bytes
available, and we assume all the 10 characters form a valid number
(with leading spaces). It's all just a way to avoid dealing with
unexpected EOF when reading the file, and to avoid not knowing in
advance how many characters long the number is.
This code still wants to parse the whole string as a single number, but
it also knows the number will end in a newline instead of nul. It
wouldn't be difficult to replace that newline with nul before parsing,
if you really want to convert this code to use a helper. But while
doing so, you have to ask yourself: does this actually make the code
any easier to understand or more correct?
I believe you are correct, and this is a good indication that blindly
inserting the replacement function is not a good idea.
The original code failed if there was a digit at offset 11 (as well as
other reasons for failing). The proposed replacement failed if offset 11
was anything other than NUL. This is different. When I said the endptr
was not needed, my proposal actually has the exact same mistake.
This does bring up a question as to whether the helper function should
eat trailing whitespace.
But also that you can't drop the wrapper in everywhere.
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
