[ovs-dev] [PATCH v2 4/6] ovs-ofctl: add output colors for flow attributes

Ben Pfaff blp at ovn.org
Tue Feb 23 23:47:51 UTC 2016


On Mon, Feb 15, 2016 at 04:21:25PM +0100, Quentin Monnet wrote:
> This commit adds colors to the “left part” of printed flows (to flow
> properties that are always present: `cookie`, `table`, timeouts, etc.).
> It uses the functions previously defined in dynamic-string.{c,h} to
> insert color markers around the names of the properties.
> 
> Signed-off-by: Quentin Monnet <quentin.monnet at 6wind.com>

Thanks for working on this.

One thing that bothers me here is the name 'color_option'.  The name and
the type (int) tell the reader roughly what it's about, but they don't
tell the reader how to interpret it.  I guess that 'bool' is more
appropriate, with a name more like 'enable_color' or just 'color'.

I see some function parameter types being written as "int const", like
here:
    void ofp_print(FILE *, const void *, size_t, int verbosity,
                   int const color_option);
The 'const' part of this isn't meaningful to the caller (even without
it, the function cannot modify the caller's data), so it should be left
out.



More information about the dev mailing list