[ovs-dev] [PATCH 3/3] smap: Return default on failure in smap_get_int/ullong.
Jan Scheurich
jan.scheurich at ericsson.com
Wed Nov 29 14:59:22 UTC 2017
Thanks for fixing this. Applying the default value is better than silently applying 0.
Even better would be to flag an error in this case...
Tested-by: Jan Scheurich <jan.scheurich at ericsson.com>
Acked-by: Jan Scheurich <jan.scheurich at ericsson.com>
> -----Original Message-----
> From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of Ilya Maximets
> Sent: Wednesday, 29 November, 2017 11:51
> To: ovs-dev at openvswitch.org
> Cc: Ilya Maximets <i.maximets at samsung.com>; Heetae Ahn <heetae82.ahn at samsung.com>
> Subject: [ovs-dev] [PATCH 3/3] smap: Return default on failure in smap_get_int/ullong.
>
> Currently smap_get_int/ullong doesn't check any conversion errors.
> Most implementations of atoi/strtoull return 0 in case of failure.
>
> This leads to returning zero in case of wrongly set database values.
> For example, commands
>
> ovs-vsctl set interface iface options:key=\"\"
> ovs-vsctl set interface iface options:key=qwe123
> ovs-vsctl set interface iface options:key=abc
>
> will have exactly same effect as
>
> ovs-vsctl set interface iface options:key=0
>
> in case where 'key' is an integer option of the iface.
> Can be checked with 'other_config:emc-insert-inv-prob' or other
> integer 'options' and 'other_config's.
>
> 0 could be not a default and not safe value for many options and
> it'll be better to return default value instead if any.
>
> Conversion functions from 'util' library used to provide proper
> error handling.
>
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
> lib/smap.c | 25 +++++++++++++++++++------
> 1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/lib/smap.c b/lib/smap.c
> index 0a5e54a..6c6717c 100644
> --- a/lib/smap.c
> +++ b/lib/smap.c
> @@ -20,6 +20,7 @@
> #include "hash.h"
> #include "openvswitch/json.h"
> #include "packets.h"
> +#include "util.h"
> #include "uuid.h"
>
> static struct smap_node *smap_add__(struct smap *, char *, void *,
> @@ -221,25 +222,37 @@ smap_get_bool(const struct smap *smap, const char *key, bool def)
> }
> }
>
> -/* Gets the value associated with 'key' in 'smap' and converts it to an int
> - * using atoi(). If 'key' is not in 'smap', returns 'def'. */
> +/* Gets the value associated with 'key' in 'smap' and converts it to an int.
> + * If 'key' is not in 'smap' or a valid integer can't be parsed from it's
> + * value, returns 'def'. */
> int
> smap_get_int(const struct smap *smap, const char *key, int def)
> {
> const char *value = smap_get(smap, key);
> + int i_value;
>
> - return value ? atoi(value) : def;
> + if (!value || !str_to_int(value, 10, &i_value)) {
> + return def;
> + }
> +
> + return i_value;
> }
>
> -/* Gets the value associated with 'key' in 'smap' and converts it to an int
> - * using strtoull(). If 'key' is not in 'smap', returns 'def'. */
> +/* Gets the value associated with 'key' in 'smap' and converts it to an
> + * unsigned long long. If 'key' is not in 'smap' or a valid number can't be
> + * parsed from it's value, returns 'def'. */
> unsigned long long int
> smap_get_ullong(const struct smap *smap, const char *key,
> unsigned long long def)
> {
> const char *value = smap_get(smap, key);
> + unsigned long long ull_value;
> +
> + if (!value || !str_to_ullong(value, 10, &ull_value)) {
> + return def;
> + }
>
> - return value ? strtoull(value, NULL, 10) : def;
> + return ull_value;
> }
>
> /* Gets the value associated with 'key' in 'smap' and converts it to a UUID
> --
> 2.7.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
More information about the dev
mailing list