[PATCH weston] Require base-10 for strtol() calls

[Bryce Harrington]

On Wed, Jul 13, 2016 at 10:27:49AM +1000, Peter Hutterer wrote:
> On Tue, Jul 12, 2016 at 04:51:27PM -0700, Bryce Harrington wrote:
> > The third arg to strtol() specifies the base to assume for the number.
> > When 0 is passed, as is currently done in option-parser.c, hexadecimal
> > and octal numbers are permitted and automatically detected and
> > converted.
> > 
> > This change is an expansion of f6051cbab84c0e577473b67f0585c0f329eb80fe
> > to cover the remaining strtol() calls in Weston, where the routine is
> > being used to read fds and pids - which are always expressed in base-10.
> > It also changes the calls in config-parser, used by
> > weston_config_section_get_int(), which in turn is being used to read
> > scales, sizes, times, rates, and delays; these are all expressed in
> > base-10 numbers only.
> > 
> > The benefit of limiting this to base-10 is to eliminate surprises when
> > parsing numbers from the command line.  Also, by making the code
> > consistent with other usages of strtol, it may make it possible to
> > factor out the common code in the future.
> > 
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> 
> code looks correct, but I didn't check all the callers, I'll just take
> your word for the base-10 assumption :)
> Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Thanks, both patches pushed:
   6351fb0..375759e  master -> master

For the strtol() calls the callers are pretty straightforward so this
change should be just fine.

Where things get a bit tricker is the strtoul() call in
weston_config_section_get_uint() which I'd also like to switch to
base-10, however some of its callers are parsing colors - which are hex
numbers and thus switching to base-10 would break those.  I haven't
sorted out what to do there.  I could introduce a new routine like
weston_config_section_get_hex() or even
weston_config_section_get_color() to handle that case, thus allowing
weston_config_section_get_uint() to be forced base-10; or I could just
leave it as-is and let it autodetect decimal or hexadecimal as needed.
Any opinions?

Bryce

> Cheers,
>    Peter
> 
> > ---
> >  compositor/main.c      | 2 +-
> >  libweston/compositor.c | 2 +-
> >  shared/config-parser.c | 2 +-
> >  xwayland/launcher.c    | 2 +-
> >  4 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/compositor/main.c b/compositor/main.c
> > index 6cf9194..8400d70 100644
> > --- a/compositor/main.c
> > +++ b/compositor/main.c
> > @@ -1684,7 +1684,7 @@ int main(int argc, char *argv[])
> >  	server_socket = getenv("WAYLAND_SERVER_SOCKET");
> >  	if (server_socket) {
> >  		weston_log("Running with single client\n");
> > -		fd = strtol(server_socket, &end, 0);
> > +		fd = strtol(server_socket, &end, 10);
> >  		if (*end != '\0')
> >  			fd = -1;
> >  	} else {
> > diff --git a/libweston/compositor.c b/libweston/compositor.c
> > index 771f1c9..96eeb17 100644
> > --- a/libweston/compositor.c
> > +++ b/libweston/compositor.c
> > @@ -4617,7 +4617,7 @@ weston_environment_get_fd(const char *env)
> >  	e = getenv(env);
> >  	if (!e)
> >  		return -1;
> > -	fd = strtol(e, &end, 0);
> > +	fd = strtol(e, &end, 10);
> >  	if (*end != '\0')
> >  		return -1;
> >  
> > diff --git a/shared/config-parser.c b/shared/config-parser.c
> > index 247e880..4c67220 100644
> > --- a/shared/config-parser.c
> > +++ b/shared/config-parser.c
> > @@ -170,7 +170,7 @@ weston_config_section_get_int(struct weston_config_section *section,
> >  	}
> >  
> >  	errno = 0;
> > -	*value = strtol(entry->value, &end, 0);
> > +	*value = strtol(entry->value, &end, 10);
> >  	if (errno != 0 || end == entry->value || *end != '\0') {
> >  		*value = default_value;
> >  		errno = EINVAL;
> > diff --git a/xwayland/launcher.c b/xwayland/launcher.c
> > index 614ef5b..b7aee3b 100644
> > --- a/xwayland/launcher.c
> > +++ b/xwayland/launcher.c
> > @@ -164,7 +164,7 @@ create_lockfile(int display, char *lockfile, size_t lsize)
> >  			return -1;
> >  		}
> >  
> > -		other = strtol(pid, &end, 0);
> > +		other = strtol(pid, &end, 10);
> >  		if (end != pid + 10) {
> >  			weston_log("can't parse lock file %s\n",
> >  				lockfile);
> > -- 
> > 1.9.1
> > 
> > _______________________________________________
> > wayland-devel mailing list
> > wayland-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/wayland-devel
> >