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 <[email protected]>

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.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to