Re: [Openocd-development] assert vs. error messages

[Rick Altherr]

On Jun 8, 2009, at 11:51 PM, Øyvind Harboe wrote:

On Tue, Jun 9, 2009 at 8:29 AM, Zach Welch<z...@superlucidity.net> wrote:

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]))
       {

-               exit(-1);
+               jtag_set_error(ERROR_JTAG_NOT_STABLE_STATE);
+               return;
       }

LOG_ERROR("BUG: TAP path doesn't finish in a stable state");

The above should be an assert().

No.  I thought this issue over carefully.  The problem with using

a bad command.  That is a _policy_.  The core JTAG module should

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

_removes_ assertions.... so you want to disable error checking for
releases, do you?

asserts is that the program should not simply exit when the users issues implement as few policies as possible, focusing on _mechanisms_. Thus, I removed the "exit" policy error and used the error reporting mechanism suddenly see OpenOCD stop running. Furthermore, compiling with - DNDEBUG

I want assertions to be compiled out when:

- they are the type of errors that only occur due to programming errors.

But who's programming error is it? An assert is appropriate if the error condition is solely contained within the library or program and can never be caused by a user. In those cases, the asserts provide sanity checking for the developers of the library or program. If a user can cause the condition, even if doing so is a programming error on their part, it should be returned as an error. Otherwise the calling program or user doesn't know what happened and any in-progress work will be lost.

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.

The problem I have with this is where do you stop? Do we check if
pointers are valid?

Anything a user can do should not cause an unexpected termination. For a program, this is limited to the actions exposed via the UI or interpreter. For a library, this is anything caused by a library entry point. If the library's API includes pointer arguments, it is considered good style to check for NULL pointers if they are invalid for that function. Otherwise the caller gets a crash at some point later that isn't necessarily easy to tie back to that arg. If you program defensively, users don't get frustrated by the "crashy piece of crap that never works." Instead they are told what is wrong so they can fix it.

I like clear programming errors to simply use asserts.

I agree when the programming error is on the part of an OpenOCD developer and not a user.

For example, this change allows an enterprising individual to begin

and scan commands for Jim (and so on... ad sanatorium).  Moreover, if

error, right?

writing a TCL replacement of the Target layer, adding low-level pathmove 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

level when it affects performance of embedded targets.

Yes, but the error checks should not necessarily be done at the very lowest

The rules of performance optimization are to measure, analyze, then optimize. Speculation isn't part of that. A few potential NOPs (untaken branches) have minimal performance impact. Now, the checks on the args should be done at the user-visible entry point. Once it has been checked there, it should be valid for anything else in the library/program. I believe the intent in this case is to expose pathmove as part of the librar API and as a TCL command. In both cases, the args should be validated in the entry point. If other internal layers use the same method _and_ the checks are measured to incur significant overhead, then the checks can be moved to a separate method that is the external entry point and the guts of the method become an internal function.

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.

Don't do that please. At least we should have a way to distinguish
between what are programming errors (asserts) and what are
real user/environment errors.

If the error occurs because of a bad arg provided by a TCL script or a library call, it's an error. If it's an invalid internal state or condition that isn't influenced by any args the user sent in, it's an assert. Nations put the guards at the border crossings, so do good programmers.

--
Øyvind Harboe
Embedded software and hardware consulting services
http://consulting.zylin.com
_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development

--
Rick Altherr
kc8...@kc8apf.net


 -- Unsigned

"He said he hadn't had a byte in three days. I had a short, so I split it with him."

smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
Openocd-development mailing list
Openocd-development@lists.berlios.de
https://lists.berlios.de/mailman/listinfo/openocd-development