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 <[email protected]> Candela Technologies Inc http://www.candelatech.com _______________________________________________ Xorp-hackers mailing list [email protected] http://mailman.ICSI.Berkeley.EDU/mailman/listinfo/xorp-hackers
