[Xorp-hackers] Using results from one XRL to respond to another

[Ben Greear]

On 03/08/2011 06:19 AM, Steven Simpson wrote:
> On 07/02/11 15:10, Steven Simpson wrote:
>>     3. Redesign the code generator so that the Xrl*TargetBase class
>>        contains methods to be provided by the implementation that receive
>>        the in-args plus a call-back for the out-args.
>
> I've just sent 3 patches to the list regarding this redesign, to allow
> asynchronous method implementations.  Please excuse any unconventional
> use of git, as I'm not used to it.  I'll summarise the changes...

Thanks for the patches.  The callback and XRL code is some tough
stuff to deal with!

I have some general comments before I review your patches in detail.

First:  It seems at least some of the 3rd patch is changing code from the
first or second patch.  This increases the amount of code review needed,
so I'm hoping you can re-work this so that the third patch is merged
into the previous patch(es).  You are welcome to have more smaller patches,
just don't have a later patch do changes to the previous ones if possible.

Second:  I *think* you are using '0' when you are passing a NULL pointer.
Please use 'NULL' instead so it's obvious it's a pointer type and not an
integer value.  I know that they are in practice equivalent on any modern
architecture...

Third:  Please compare the size of the stripped binaries before and after
your changes.  If your changes cause a significant increase in disk space
usage, we may want to allow them to be #ifdef'd out:  There are folks who
are trying to use xorp with very small storage systems.  You can check
the SConstruct file for examples of how to compile out things (see 'disable_profile', etc).

Fourth:  Please add more verbose descriptions to the individual patches.

And finally:  I'd like a:

Signed-off-by: [you] <your-email>

on each patch.  This basically says you are contributing this patch
to the project under the GPL and you have a right to do so.

See:  http://gerrit.googlecode.com/svn/documentation/2.0/user-signedoffby.html

Use:  git commit -s
to have it automatically appended, and the git format-patch can also append
it automatically I believe.

Please repost your patches when you are satisfied with them, and/or
resolve that the suggestions above are not needed for whatever reason.

Thanks,
Ben

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