Skip to content

Commit

Permalink
smap: Return default on failure in smap_get_int/ullong.
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
Tested-by: Jan Scheurich <[email protected]>
Acked-by: Jan Scheurich <[email protected]>
  • Loading branch information
igsilya authored and blp committed Nov 29, 2017
1 parent c24b345 commit 01393f0
Showing 1 changed file with 19 additions and 6 deletions.
25 changes: 19 additions & 6 deletions lib/smap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 *,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 01393f0

Please sign in to comment.