Looks good. Thanks for fixing Coverity issue #10712, too. :-)
--Justin
On Feb 22, 2011, at 4:28 PM, Ben Pfaff wrote:
> Some actions checked that 'arg' was nonnull before attempting to parse it
> but a lot of them didn't. This commit avoids the segfault by substituting
> an empty string when no argument is given. It also updates a few of the
> action implementations to correspond.
>
> Reported-by: Reid Price <r...@nicira.com>
> Bug #4462.
> ---
> lib/ofp-parse.c | 14 +++++++++-----
> 1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
> index 3fac474..73c3ebc 100644
> --- a/lib/ofp-parse.c
> +++ b/lib/ofp-parse.c
> @@ -43,7 +43,7 @@ str_to_u32(const char *str)
> char *tail;
> uint32_t value;
>
> - if (!str) {
> + if (!str[0]) {
> ovs_fatal(0, "missing required numeric argument");
> }
>
> @@ -61,6 +61,10 @@ str_to_u64(const char *str)
> char *tail;
> uint64_t value;
>
> + if (!str[0]) {
> + ovs_fatal(0, "missing required numeric argument");
> + }
> +
> errno = 0;
> value = strtoull(str, &tail, 0);
> if (errno == EINVAL || errno == ERANGE || *tail) {
> @@ -271,6 +275,7 @@ str_to_action(char *str, struct ofpbuf *b)
> pos = str;
> n_actions = 0;
> for (;;) {
> + char empty_string[] = "";
> char *act, *arg;
> size_t actlen;
> uint16_t port;
> @@ -320,7 +325,7 @@ str_to_action(char *str, struct ofpbuf *b)
> pos = arg + arglen;
> } else {
> /* There might be no argument at all. */
> - arg = NULL;
> + arg = empty_string;
> pos = act + actlen + (act[actlen] != '\0');
> }
> act[actlen] = '\0';
> @@ -410,7 +415,7 @@ str_to_action(char *str, struct ofpbuf *b)
> nan->subtype = htons(NXAST_NOTE);
>
> b->size -= sizeof nan->note;
> - while (arg && *arg != '\0') {
> + while (*arg != '\0') {
> uint8_t byte;
> bool ok;
>
> @@ -472,7 +477,7 @@ str_to_action(char *str, struct ofpbuf *b)
>
> /* Unless a numeric argument is specified, we send the whole
> * packet to the controller. */
> - if (arg && (strspn(arg, "0123456789") == strlen(arg))) {
> + if (arg[0] && (strspn(arg, "0123456789") == strlen(arg))) {
> oao->max_len = htons(str_to_u32(arg));
> } else {
> oao->max_len = htons(UINT16_MAX);
> @@ -909,4 +914,3 @@ parse_ofp_flow_stats_request_str(struct
> flow_stats_request *fsr,
> fsr->out_port = fm.out_port;
> fsr->table_id = table_id;
> }
> -
> --
> 1.7.1
>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
