[PATCH v1] Added string conversion utility functions
Bill Spitzak
spitzak at gmail.com
Mon Dec 1 10:30:04 PST 2014
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.
More information about the wayland-devel
mailing list