Greg Hudson <ghudson@MIT.EDU> writes:
> > 3. Make svn_error_clear() a macro like this:
>
> Still yech, in my opinion. And it would violate our API compatibility
> rules, unless we gave it a new name.
We could give it a new name, that would be easy. Have to change a lot
of call sites, but no big deal.
> In my opinion, code at the end of a function to "clean up the error we
> might have encountered at some undefine point earlier on" is inelegant
> and error-prone with or without safeties like this. It's not always
> 100% clear how to avoid that sort of construct without duplicating a lot
> of code. But I don't think we should be making project-wide allowances
> for the sake of the few places we have that construct.
So your solution is to avoid code like that? I'm sure we can catch it
in review from now on if we want. But there are a lot of existing
places to clean up, and in some of them I'm not sure I agree with the
"inelegant" assessment. There's a reason C has 'goto'. (On the other
hand, the presence of the bug we just fixed is a good argument against
the construct.)
I think your solution boils down to (1): Audit all current uses, try
to be smarter in the future.
> > 4. Mike's interesting idea: Have svn_error_clear set all the fields
> > in the error to zero, so that if we try anything at all with the
> > error later, we'll get a quick seg fault.
>
> That's fine with me.
I'm not sure the benefit is worth the effort; we'd still want to do
(1) anyway.
> > But it looks like if SVN_DEBUG_ERROR() is defined, then we first cdr
> > down to the end of the error chain, and only clear that last error.
> > All the ones preceding it will not be touched.
>
> All of the errors in a chain live in the same pool.
Thank you, that makes it click for me.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Dec 16 02:15:55 2004