On Wed, Jun 24, 2015 at 1:11 PM, Ben Pfaff <b...@nicira.com> wrote:
> On Fri, Jun 19, 2015 at 04:13:21PM -0700, Jesse Gross wrote:
> In parse_ofp_geneve_table_mod_str(), I'd prefer to use %i instead of %x,
> because one of my long-term goals for OVS is to switch away from this
> awful scanf() stuff to a proper parser and having hex numbers without 0x
> prefixes will make that a lot harder.

Yeah, this is actually just a mistake - the hex values should be
prefixed with 0x.

> This ovs-ofctl syntax is going to trip up anyone who isn't careful about
> shell quoting, both for {a,b,c} and for the > in the syntax.  I guess
> that's OK but the example in the ovs-ofctl manpage should really show
> single or double quotes in my opinion.

I added the quotes to the examples in the man page. I wouldn't mind
having a syntax that doesn't require quoting but this seemed somewhat
descriptive (to me at least) and consistent with our other fields.

> I think decode_geneve_table_mappings() has a memory leak in the error
> case.  You could fix it by doing the list_push_back() immediately
> instead of only following validation.

Oops - thanks for catching that.

> It would be kind to include in the documentation the minimum required
> OVS version and that this is an Open vSwitch extension.

Yes, I added that.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to