On Thu, 2009-06-11 at 11:25 +0200, Øyvind Harboe wrote: > > assert() simply does not seem correct for this kind of check; you are > > not checking for programming errors. These checks are for potential > > run-time errors -- particularly now that you have added a Jim command to > > call this function directly. > > Read my patch carefully: the assert() only runs *AFTER* the check > has happened. > > Why? > > jtag_add_pathmove() is the one that does all sanity check runtime > and reports an error. No exit() upon user error here. > > jtag_add_pathmove_nocheck() is the to-the-metal version that > is used in code that *does not* have any user input arguments > and where the code *will* pass in correct arguments, e.g. inside > xscale.c.
Ugh. Okay, I see this now. I missed the actual problem: the normal invocation calls jtag_check_pathmove twice. That's not good, even if the second one will never fail thanks to the first one. The tail of your _nocheck variant can be put into 'jtag_add_pathmove_core', then you can call that from the plain and _nocheck variants. You would also need to add a forward declaration in the header, as we will _definitely_ need to documentation that makes their differences absolutely crystal clear to developers. Honestly, I am not really in favor of this from the perspective that it is a poor library API. It does nothing to improve correctness and actually opens the door for developers to use the static checking version in a context where run-time errors may occur. I would rather first see some benchmark numbers showing how the new version performs better. Here, performance seems like another windmill. ;) --Z _______________________________________________ Openocd-development mailing list [email protected] https://lists.berlios.de/mailman/listinfo/openocd-development
