On Wed, Nov 12, 2014 at 01:52:57PM +0900, Simon Horman wrote: > 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.
Thanks! _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
