On Tue, Feb 08, 2011 at 01:27:48PM -0800, Ethan Jackson wrote:
> Looks Good. Some minor comments below.
>
> > @@ -0,0 +1,518 @@
> > +/*
> > + * Copyright (c) 2009, 2010, 2011 Nicira Networks.
> > + *
> Should this only be 2011?
This new file is extracted out of ovsdb-client.c, which had all those
copyright years, so it is my understanding that I should keep them when
I move out that code. IANAL so I could be wrong...
> +static const char *
> +cell_to_text(const struct cell *cell_, const struct table_style *style)
> +{
> + struct cell *cell = (struct cell *) cell_;
>
> I don't see a reason to make cell_ const as nothing relies on it being const
> and it is not in-fact const in this context.
Fair enough, changed.
> + * cell actually contains OVSDB data because 'text' cannot be formatted
> + * according to the table style; it is always output verbatim.
> + */
> "output verbatim" -> "outputted verbatim"
"outputted" is really ugly. The dictionaries I've looked in
(wiktionary, Merriam-Webster) say that "output" is also correct.
>
> + if (length > widths[x])
> + widths[x] = length;
>
> This should have braces.
Thanks, fixed.
> +struct cell *table_add_cell(struct table *table);
> I think this should be: struct cell *table_add_cell(struct table *);
Thanks, fixed.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
