On Mon, Mar 16, 2015 at 12:01:56PM -0400, Russell Bryant wrote:
> I started working on a new command line utility that used this shared
> code. I wanted the ability to pass some data from common
> initialization code to all of the commands. You can find a similar
> pattern in ovs-vsctl.
>
> This patch updates the command handler to take a new struct,
> ovs_cmdl_context, instead of argc and argv directly. It includes argc
> and argv, but also includes an opaque type (void *), where the user of
> this API can attach its custom data it wants passed along to command
> handlers.
>
> This patch affected the ovstest sub-programs, as well. The patch
> includes a bit of an odd hack to OVSTEST_REGISTER() to avoid making
> the main() function of the sub-programs take a ovs_cmdl_context.
> The test main() functions still receive argc and argv directly, as
> that seems more natural. The test-subprograms themselves are able to
> make use of a context internally, though.
>
> Signed-off-by: Russell Bryant <[email protected]>
Thanks for the patch!
I'm happy with this idea, although in writing it myself I would have a
hard time deciding whether to include the void * pointer or to expect
the caller to embed the structure into a larger one that could contain
additional data.
In ovstest.h I'd prefer to use a __ suffix rather than prefix, since
underscore prefixes are reserved to the C implementation.
Clang complains about a number of the initializers:
../utilities/ovs-benchmark.c:84:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
../ovsdb/ovsdb-tool.c:58:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
1 error generated.
make[2]: *** [utilities/ovs-benchmark.o] Error 1
1../tests/test-ovsdb.c:61:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
error generated.
make[2]: *** [ovsdb/ovsdb-tool.o] Error 1
make[2]: *** [tests/test-bitmap.o] Error 1
../tests/test-jsonrpc.c:43:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
../tests/test-ovsdb.c:1545:53: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context transact_ctx = { 0, };
^
1 error generated.
make[2]: *** [tests/test-jsonrpc.o] Error 1
2 errors generated.
make[2]: *** [tests/test-ovsdb.o] Error 1
../utilities/ovs-ofctl.c:120:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
../tests/test-util.c:1107:40: error: missing field 'argv' initializer
[-Werror,-Wmissing-field-initializers]
struct ovs_cmdl_context ctx = { 0, };
^
1 error generated.
Clang doesn't complain about { .argc = 0 } as an initializer.
Clang and GCC both complain about an incompatible function:
../tests/test-bitmap.c:153:5: error: initialization from incompatible
pointer type [-Werror]
../tests/test-bitmap.c:153:5: error: (near initialization for
'commands[1].handler') [-Werror]
and
../tests/test-bitmap.c:153:31: error: incompatible pointer types
initializing
'ovs_cmdl_handler' (aka 'void (*)(struct ovs_cmdl_context *)') with an
expression of type 'void (int, char **)'
[-Werror,-Wincompatible-pointer-types]
{"benchmark", NULL, 1, 1, run_benchmarks},
^~~~~~~~~~~~~~
1 error generated.
Would you mind fixing that up?
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
