[Xorp-hackers] [PATCH] Supporting asynchronous method implementations

[Ben Greear]

On 03/11/2011 05:36 AM, ss at comp.lancs.ac.uk wrote:
> From: Steven Simpson<ss at comp.lancs.ac.uk>
>
> * Command map and dispatcher declare types for callbacks to pass
>    error code and out-arguments.
>
> * Handlers in generated targets meet these callback signatures.
>
> * Various dispatch methods changed so that out-arguments and
>    error codes are removed from the signature, and the appropriate
>    callback type is added, all to track changes to command map and
>    dispatcher.
>
> * Generated targets include default asynchronous implementations
>    which call synchronous (pure virtual) methods.
>
> * One finder method FinderClient::dispatch_tunneled_xrl is
>    implemented asynchronously.
>
> * FinderClient made a friend of XrlCmdError so it can use the
>    private constructor.
>
> * All changes compiled in only if XORP_ENABLE_ASYNC_SERVER is defined,
>    as set by enable_async_server=True on scons.

This looks a lot better.  There are a few things that bother me,
but not sure there is a better way to do it:

* I don't like the typedef'd return values that change based on
enabling/disabling the async-server.  Maybe we can use a helper
method somehow?  I don't mind if enabling async-server adds a small bit
of code/size to the binaries, so not everything has to be #ifdef'd
out, for instance.

I'm guessing the vast majority of the extra binary size comes from
the auto-generated code, so the #ifdefs in it provide most of the
benefits.

* The Macro that returns one type or another should be all caps
  at the least.  It will be confusing either way, but at least all-caps
  gives the reader a clue it's a macro instead of a standard function.

Have you run 'scons check' with and without this enabled?  It takes
a few hours to complete, but likely it will catch obvious errors with
XRL transport as almost every test uses XRLs one way or another.

Thanks,
Ben

-- 
Ben Greear <greearb at candelatech.com>
Candela Technologies Inc  http://www.candelatech.com