[ovs-dev] [PATCH] lib: ofp-parse segfaults if required argument isn't specified
Ben Pfaff
blp at nicira.com
Fri Oct 8 23:33:12 UTC 2010
Sure, that's clear.
On Fri, Oct 08, 2010 at 04:20:03PM -0700, Ethan Jackson wrote:
> alright, how about "missing required numeric argument"
>
> Ethan
>
> On Fri, Oct 8, 2010 at 4:00 PM, Ben Pfaff <blp at nicira.com> wrote:
> > [Adding dev at openvswitch.org back]
> >
> > The fact that I missed here is that str_to_u32() is implemented in
> > lib/ofp-parse.c. I thought it was one of the similarly named but
> > different functions in lib/util.c.
> >
> > It's fine now that I see that, although I'd prefer that the message said
> > something about a missing number, since "<NULL>" won't mean anything to
> > most users.
> >
> > Thanks,
> >
> > Ben.
> >
> > On Fri, Oct 08, 2010 at 03:52:21PM -0700, Ethan Jackson wrote:
> >> Well it gets passed in if they don't supply a numeric argument in the
> >> ofproto string when one is required. For expample "in_port=3
> >> actions=resubmit" instead of "in_port=3 actions=resubmit:4"
> >>
> >> I think this is the right place to catch it. If you look at that
> >> function (with the patch applied)
> >>
> >> static uint32_t
> >> str_to_u32(const char *str)
> >> {
> >> char *tail;
> >> uint32_t value;
> >>
> >> if (!str) {
> >> ovs_fatal(0, "invalid numeric format <NULL>");
> >> }
> >>
> >> errno = 0;
> >> value = strtoul(str, &tail, 0);
> >> if (errno == EINVAL || errno == ERANGE || *tail) {
> >> ovs_fatal(0, "invalid numeric format %s", str);
> >> }
> >> return value;
> >> }
> >>
> >> The function is already responsible for checking for an "invalid
> >> numeric format". I think the empty number is a special kind of
> >> "invalid numeric format".
> >>
> >> On Fri, Oct 8, 2010 at 3:40 PM, Ben Pfaff <blp at nicira.com> wrote:
> >> > On Fri, Oct 08, 2010 at 03:17:44PM -0700, Ethan Jackson wrote:
> >> >> Running ovs-ofctl add-flow br0 "in_port=3 actions=resubmit" would
> >> >> segfault instead of reporting an error.
> >> >
> >> > This seems like an odd way to fix the problem. Why is NULL getting
> >> > passed in? I'd rather fix that.
> >> >
> >
More information about the dev
mailing list