On Wed, 2009-11-11 at 12:04 -0700, David Brownell wrote: > > +#define CALL_COMMAND_HANDLER(name, extra...) \ > > + name(cmd_ctx, cmd, args, argc, ##extra) > > That one I don't like. It relies on lexical capture > for most parameters, and doesn't even use "name".
name is right up there at the front.... > Lexical capture is one of the classic Things To Avoid > for C macros ... a Dirty Trick, which makes code hard > to read and evolve. It's clearer to just pass the > params explicitly. Yes. It's dirty. The remaining patches also make the code cleaner. I know it's something not to do -- in general. But it works, and the code looks really pretty. The fact is that it cannot be screwed up without deliberately overriding the parameters (which should only ever be defined by these other macros), which could not be done at the top-level of a handler. It's unlikely to ever be a problem. As long as it's documented fully, I see this as a valid technique. This is a very extensively used paradigm. Personally, I think this could very well be the only exception worth making in the tree. I'm not condoning it as a general solution, no. I once heard that one should never use goto either, but there are exceptions for that rule too. ;) > > +/** > > + * Always use this macro to define new command handler functions. > > + * It ensures the parameters are ordered, typed, and named properly, so > > + * they be can be used by other macros (e.g. COMMAND_PARSE_NUMBER). > > + * All command handler functions must be defined as static in scope. > > + */ > > +#define COMMAND_HANDLER(name) static __COMMAND_HANDLER(name) > > Why is this the only one that has a "static"? To ensure they are always static. After this series is applied, _all_ command handlers will be static: guaranteed. > > +/** > > + * Similar to COMMAND_HANDLER, except some parameters are expected. > > + * globally-scoped > > + */ > > +#define COMMAND_HELPER(name, extra...) __COMMAND_HANDLER(name, extra) > > ... seems this one should have it too. See patch 7/20. The s3c24xx_nand module provides a non-static helper. Otherwise, yes, they could be the same. Cheers, Zach _______________________________________________ Openocd-development mailing list Openocd-development@lists.berlios.de https://lists.berlios.de/mailman/listinfo/openocd-development
