On Wed, Aug 15, 2012 at 1:20 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Wed, Aug 15, 2012 at 11:51:12AM -0700, Mehak Mahajan wrote:
> > On Wed, Aug 15, 2012 at 11:23 AM, Ben Pfaff <b...@nicira.com> wrote:
> >
> > > On Wed, Aug 15, 2012 at 12:15:58AM -0700, Mehak Mahajan wrote:
> > > > --- a/include/openflow/nicira-ext.h
> > > > +++ b/include/openflow/nicira-ext.h
> > > > @@ -291,7 +291,8 @@ enum nx_action_subtype {
> > > > NXAST_OUTPUT_REG, /* struct nx_action_output_reg */
> > > > NXAST_LEARN, /* struct nx_action_learn */
> > > > NXAST_EXIT, /* struct nx_action_header */
> > > > - NXAST_DEC_TTL, /* struct nx_action_header */
> > > > + NXAST_DEC_TTL, /* struct nx_action_cnt_ids */
> > >
> > > It doesn't make sense to change NXAST_DEC_TTL. Software already
> > > exists that knows that NXAST_DEC_TTL is just an nx_action_header. We
> > > can't break that software.
> > >
> > >
> >
> > It seems I misunderstood what you meant the first time. I will change
> back
> > NXAST_DEC_TTL to its earlier behavior.
>
> Sorry. There are lots of layers these days. Maybe I was not clear
> enough. I don't mean to cause more work than necessary.
>
> > > > + cntr = strtok_r(string, ", ", &idstr);
> > > > +
> > > > + for (;;) {
> > > > + uint16_t id;
> > > > +
> > > > + id = atoi(cntr);
> > >
> > > I don't think it's safe to use 'cntr' without testing it here. I
> > > believe that it could be NULL even on the first iteration.
> > >
> > >
> > Makes sense.
> > As a side note, there is a check on the top to see if *arg == '\0' which
> is
> > truly the only case in which cntr would be null. In every other case cntr
> > would return the string in the () ... I also tested the code with just
> > dec_ttl(), and it returned *arg = '\0' and hence went into the
> > NXAST_DEC_TTL code snippet.
>
> I think that syntax errors such as dec_ttl(,) would also yield NULL.
> Did you try that?
>
Yes. I tried out dec_ttl(,) after your comments, and it yields a segfault.
Thanks. I will be sure to try out all these cases before sending out the
next patch.
>
> > I will rework the patch and send it out according to your latest
> comments.
>
> Thanks!
>
Thanks for your help Ben.
Thanks!
Mehak
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
