[ovs-dev] [PATCH v2 09/13] ofp-util: Enhanced command error logging during encoding group mod messages

Simon Horman simon.horman at netronome.com
Wed Nov 12 04:52:57 UTC 2014


On Tue, Nov 11, 2014 at 08:44:29PM -0800, Ben Pfaff wrote:
> On Wed, Nov 12, 2014 at 12:46:08PM +0900, Simon Horman wrote:
> > On Tue, Nov 11, 2014 at 09:21:36AM -0800, Ben Pfaff wrote:
> > > Using a blind array dereference into cmd_str[] in bad_group_cmd() seems
> > > like it could easily be overlooked if new commands get added later.  I
> > > would use a switch statement or otherwise make this more easily
> > > maintainable.
> > 
> > Sure, will do.
> > 
> > I have a feeling that I used this approach here and elsewhere in the
> > series to match the style of other open-vswtich code. I'd be happy
> > to do a pass on cleaning-up other instances of this approach if you
> > are interested.
> 
> I did spot some other instances of the same thing nearby the code that
> you added.

Thanks, perhaps it was originally my idea after all (I honestly don't
recall my reasoning behind the older code).

> It would be nice to clean those up too.  If you decide to
> work on it, thank you!  (I don't insist on a switch statement, by the
> way.  Sometimes a build assertion or something else is more appropriate.
> But I don't like code that breaks silently when someone adds an enum.)

Understood. I'll add this as a low-priority task.
And I won't mind at all if someone else gets there first.



More information about the dev mailing list