[PATCH weston v3 4/5] Add safe_strtoint() helper

[Eric Engestrom]

On Mon, Aug 01, 2016 at 12:48:21PM +1000, Peter Hutterer wrote:
> On Fri, Jul 29, 2016 at 09:43:59AM -0700, Bryce Harrington wrote:
> > Adds a safe strtol helper function, modeled loosely after Wayland
> > scanner's strtouint.  This encapsulates the various quirks of strtol
> > behavior, and streamlines the interface to just handling base-10 numbers
> > with a simple true/false error indicator and a uint32_t return by
> > reference.
> > 
> > Signed-off-by: Bryce Harrington <bryce at osg.samsung.com>
> > Reviewed-by: Thiago Macieira <thiago.macieira at intel.com>
> > ---
> >  shared/string-helpers.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 78 insertions(+)
> >  create mode 100644 shared/string-helpers.h
> > 
> > diff --git a/shared/string-helpers.h b/shared/string-helpers.h
> > new file mode 100644
> > index 0000000..0617229
> > --- /dev/null
> > +++ b/shared/string-helpers.h
> > @@ -0,0 +1,78 @@
> > +/*
> > + * Copyright © 2016 Samsung Electronics Co., Ltd
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining
> > + * a copy of this software and associated documentation files (the
> > + * "Software"), to deal in the Software without restriction, including
> > + * without limitation the rights to use, copy, modify, merge, publish,
> > + * distribute, sublicense, and/or sell copies of the Software, and to
> > + * permit persons to whom the Software is furnished to do so, subject to
> > + * the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the
> > + * next paragraph) shall be included in all copies or substantial
> > + * portions of the Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT.  IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef WESTON_STRING_HELPERS_H
> > +#define WESTON_STRING_HELPERS_H
> > +
> > +#ifdef	__cplusplus
> > +extern "C" {
> > +#endif
> 
> do we need this in an internal helper?
> 
> > +
> > +#include <stdbool.h>
> > +#include <stdlib.h>
> > +#include <errno.h>
> > +#include <assert.h>
> > +
> > +/* Convert string to integer
> > + *
> > + * Parses a base-10 number from the given string.  Checks that the
> > + * string is not blank, contains only numerical characters, and is
> > + * within the range of -INT_MAX to INT_MAX.  If the validation is

s/-INT_MAX/INT_MIN/, and if I'm being pedantic it should be
"INT32_MIN to INT32_MAX" since you're using int32_t, not int :)

> > + * successful the result is stored in *value; otherwise *value is
> > + * unchanged and errno is set appropriately.
> > + *
> > + * \return true if number parsed successfully, false on error
> 
> "if the number". IMO you should squash the safe_strtoint() tests into this
> patch and have a separate patch for switching the existing calls over. 5/5
> looks like a rebase gone wrong.
> 
> with that, Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

Agreed, and you can also have my r-b (for the whole series):
Reviewed-by: Eric Engestrom <eric.engestrom at imgtec.com>

> 
> Cheers,
>    Peter
> 
> > + */
> > +static inline bool
> > +safe_strtoint(const char *str, int32_t *value)
> > +{
> > +	long ret;
> > +	char *end;
> > +
> > +	assert(str != NULL);
> > +
> > +	errno = 0;
> > +	ret = strtol(str, &end, 10);
> > +	if (errno != 0) {
> > +		return false;
> > +	} else if (end == str || *end != '\0') {
> > +		errno = EINVAL;
> > +		return false;
> > +	}
> > +
> > +	*value = (int32_t)ret;
> > +	if ((long)*value != ret) {
> > +		errno = ERANGE;
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +#ifdef	__cplusplus
> > +}
> > +#endif
> > +
> > +#endif /* WESTON_STRING_HELPERS_H */
> > -- 
> > 1.9.1
> >