[ovs-dev] [PATCH 6/6] ofp-print: Move much of the printing code into message-specific files.

Ben Pfaff blp at ovn.org
Wed Mar 14 18:41:46 UTC 2018


On Tue, Mar 13, 2018 at 04:15:08PM -0700, Justin Pettit wrote:
> 
> 
> > On Mar 13, 2018, at 4:04 PM, Justin Pettit <jpettit at ovn.org> wrote:
> > 
> > 
> >> On Feb 16, 2018, at 2:54 PM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > It looks like this was mostly moving code around, so I didn't pore over the review, but let me know if you want me to take a closer look.  I did notice a few smaller things:
> > 
> >> diff --git a/lib/ofp-table.c b/lib/ofp-table.c
> >> index 558e4bcd9127..7df7167deddb 100644
> >> --- a/lib/ofp-table.c
> >> +++ b/lib/ofp-table.c
> >> ...
> >> +const char *
> >> +ofputil_table_eviction_to_string(enum ofputil_table_eviction eviction)
> >> +{
> >> +    switch (eviction) {
> >> +    case OFPUTIL_TABLE_EVICTION_DEFAULT: return "default";
> >> +    case OFPUTIL_TABLE_EVICTION_ON: return "on";
> >> +    case OFPUTIL_TABLE_EVICTION_OFF: return "off";
> >> +    default: return "***error***";
> >> +    }
> >> +
> >> +}
> > 
> > This seems to have an unnecessary blank line.
> > 
> >> +const char *
> >> +ofputil_table_vacancy_to_string(enum ofputil_table_vacancy vacancy)
> >> +{
> >> +    switch (vacancy) {
> >> +    case OFPUTIL_TABLE_VACANCY_DEFAULT: return "default";
> >> +    case OFPUTIL_TABLE_VACANCY_ON: return "on";
> >> +    case OFPUTIL_TABLE_VACANCY_OFF: return "off";
> >> +    default: return "***error***";
> >> +    }
> >> +
> >> +}
> > 
> > Same here.
> > 
> >> +void
> >> +ofputil_table_desc_format(struct ds *s, const struct ofputil_table_desc *td,
> >> +                          const struct ofputil_table_map *table_map)
> >> +{
> >> ...
> >> +}
> >> +
> >> +
> >> /* This function parses Vacancy property, and decodes the
> > 
> > And here.
> > 
> >> /* Convert 'setting' (as described for the "mod-table" command
> >> * in ovs-ofctl man page) into 'tm->table_vacancy->vacancy_up' and
> >> * 'tm->table_vacancy->vacancy_down' threshold values.
> >> * For the two threshold values, value of vacancy_up is always greater
> >> * than value of vacancy_down.
> >> *
> >> - * Returns NULL if successful, otherwise a malloc()'d string describing the
> >> + * Returns NULL if successful, otherwise a malloc()'d s describing the
> > 
> > I assume truncating "string" to "s" was unintentional.
> > 
> > Thanks for tackling "ofp-print.c".
> > 
> > --Justin
> 
> Oh, and:
> 
> Acked-by: Justin Pettit <jpettit at ovn.org>
> 
> --Justin
> 
> 

Thanks.  I fixed up all of that and I'll apply this series to master in
a few minutes.


More information about the dev mailing list