[ovs-dev] [PATCH 3/3] command-line: add ovs_cmdl_context

Russell Bryant rbryant at redhat.com
Mon Mar 16 21:16:57 UTC 2015


On 03/16/2015 05:08 PM, Ben Pfaff wrote:
> 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 <rbryant at redhat.com>
> 
> 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.

Hm, sure, that works too.  I can't say I have much of a preference.  I'd
be happy to change to the other if you'd prefer.

I suppose embedding is slightly more efficient, though I don't expect
any of this to be in performance critical code paths.

<snip list of things to fix>

> Would you mind fixing that up?

No problem at all.  Thanks for the feedback!  I'll also start testing
builds with both gcc and clang, instead of just gcc.  I wonder how I
missed the error ... I'll get it fixed up in any case.

-- 
Russell Bryant



More information about the dev mailing list