[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