On Tue, 2009-06-09 at 07:57 +0200, Øyvind Harboe wrote:
> How about a clearer policy of using assert()'s?
>
> I'm thinking that error()'s should be reserved for "real" runtime errors.
>
>
> > if (!tap_is_state_stable(path[num_states - 1]))
> > {
> > LOG_ERROR("BUG: TAP path doesn't finish in a stable state");
> > - exit(-1);
> > + jtag_set_error(ERROR_JTAG_NOT_STABLE_STATE);
> > + return;
> > }
>
>
> The above should be an assert().
No. I thought this issue over carefully. The problem with using
asserts is that the program should not simply exit when the users issues
a bad command. That is a _policy_. The core JTAG module should
implement as few policies as possible, focusing on _mechanisms_. Thus,
I removed the "exit" policy error and used the error reporting mechanism
to make the pathmove mechanism work in more contexts.
If it will be possible for the user to script a call such that their
typos get translated to a bad pathmove call, then they should not
suddenly see OpenOCD stop running. Furthermore, compiling with -DNDEBUG
_removes_ assertions.... so you want to disable error checking for
releases, do you?
By deferring the policy decision to the caller, the _program_ can choose
to live or die when a potentially user-supplied parameter was incorrect.
This is the right move for creating a reusable JTAG library, as you must
assume the user will do things with the core that you did not intend.
Basically, the JTAG library should not make policy to abort, if it will
be used by script commands. While this was effectively an 'assert'
before my changes, no one reported trouble with it. Further, none of
the higher layers appear to check jtag_error anyway, so this should not
cause unexpected new errors to appear from the new error codes.
For example, this change allows an enterprising individual to begin
writing a TCL replacement of the Target layer, adding low-level pathmove
and scan commands for Jim (and so on... ad sanatorium). Moreover, if
any command that builds on this function were to be able to be similarly
misused, the program should not exit -- the interpreter should report an
error, right?
Cheers,
Zach
P.S. In raising this point, you have exposed the fact that assert has
been used several throughout the JTAG module incorrectly. Expect a
flurry of patches to fix these bugs this soon.
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development
