[ovs-dev] [ovs-discuss] [ACLv2 13/19] flow: Move wildcarded flow printing out of ofp-print.

Jesse Gross jesse at nicira.com
Fri Sep 4 23:55:52 UTC 2009



Ben Pfaff wrote:
> Jesse Gross <jesse at nicira.com> writes:
>
>   
>> --- a/lib/flow.c
>> +++ b/lib/flow.c
>> @@ -313,12 +313,14 @@ void
>>  flow_format(struct ds *ds, const flow_t *flow)
>>  {
>>      ds_put_format(ds, "port%04x:vlan%d mac"ETH_ADDR_FMT"->"ETH_ADDR_FMT" "
>> -                  "type%04x proto%"PRId8" ip"IP_FMT"->"IP_FMT" port%d->%d",
>> +                  "type%04x proto%"PRId8" ip"IP_FMT"->"IP_FMT" port%d->%d "
>> +                  "arp"ETH_ADDR_FMT"->"ETH_ADDR_FMT,
>>                    flow->in_port, ntohs(flow->dl_vlan),
>>                    ETH_ADDR_ARGS(flow->dl_src), ETH_ADDR_ARGS(flow->dl_dst),
>>                    ntohs(flow->dl_type), flow->nw_proto,
>>                    IP_ARGS(&flow->nw_src), IP_ARGS(&flow->nw_dst),
>> -                  ntohs(flow->tp_src), ntohs(flow->tp_dst));
>> +                  ntohs(flow->tp_src), ntohs(flow->tp_dst),
>> +                  ETH_ADDR_ARGS(flow->ar_sha), ETH_ADDR_ARGS(flow->ar_tha));
>>  }
>>     
>
> This is getting really unreadable.  How about breaking it up into
> pieces, e.g.
>
>   ds_put_format(ds, "port%04x:vlan%d "
>                     "mac"ETH_ADDR_FMT"->"ETH_ADDR_FMT" "
>                     "type%04x "
>                     "proto%"PRId8" "
>                     "ip"IP_FMT"->"IP_FMT" "
>                     "port%d->%d "
>                     "arp"ETH_ADDR_FMT"->"ETH_ADDR_FMT,
>                     flow->in_port, ntohs(flow->dl_vlan),
>                     ETH_ADDR_ARGS(flow->dl_src), ETH_ADDR_ARGS(flow->dl_dst),
>                     ntohs(flow->dl_type),
>                     flow->nw_proto,
>                     IP_ARGS(&flow->nw_src), IP_ARGS(&flow->nw_dst),
>                     ntohs(flow->tp_src), ntohs(flow->tp_dst),
>                     ETH_ADDR_ARGS(flow->ar_sha), ETH_ADDR_ARGS(flow->ar_tha));
>
>   

I agree this is cleaner, so I broke it up.

>> +static void print_wild(struct ds *string, const char *leader, int is_wild,
>> +            int verbosity, const char *format, ...) 
>> +            __attribute__((format(printf, 5, 6)));
>>     
>
> Please use the PRINTF_FORMAT macro from compiler.h.
>
> Ah, I see that this is just code being moved around.  Oh well,
> the comment still applies.
>   

I made the change.





More information about the dev mailing list