Sounds good. Ethan
On Tue, Feb 8, 2011 at 4:09 PM, Ben Pfaff <[email protected]> wrote: > 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
