Re: [PATCH weston v3 4/5] Add safe_strtoint() helper
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 > > Reviewed-by: Thiago Macieira > > --- > > 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 000..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 > > +#include > > +#include > > +#include > > + > > +/* 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 Agreed, and you can also have my r-b (for the whole series): Reviewed-by: Eric Engestrom > > 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 > > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston v3 4/5] Add safe_strtoint() helper
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 > Reviewed-by: Thiago Macieira > --- > 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 000..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 > +#include > +#include > +#include > + > +/* 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 > + * 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 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 > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[PATCH weston v3 4/5] Add safe_strtoint() helper
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 Reviewed-by: Thiago Macieira --- 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 000..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 + +#include +#include +#include +#include + +/* 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 + * 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 + */ +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 ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel