[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r933272 - in /subversion/trunk/subversion/libsvn_wc: update_editor.c wc_db.c

From: Julian Foad <julian.foad_at_wandisco.com>
Date: Mon, 12 Apr 2010 18:11:21 +0100

Greg Stein wrote:
> On Mon, Apr 12, 2010 at 12:35, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> > On Mon, 2010-04-12 at 11:48 -0400, Greg Stein wrote:
> >> On Mon, Apr 12, 2010 at 11:19, <julianfoad_at_apache.org> wrote:
> >> >...
> >...
> >> > + SVN_ERR(svn_sqlite__reset(stmt));
> >> > + return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, NULL,
> >> > + _("The pristine text with checksum '%s' was "
> >> > + "not found"),
> >> > + svn_checksum_to_cstring_display(sha1_checksum,
> >> > + scratch_pool));
> >> > }
> >>
> >> You could write it as:
> >>
> >> return svn_error_createf(ERR, svn_sqlite__reset(stmt), ...);
> >>
> >> *shrug*
> >
> > I think nesting an error normally implies that the nested error was the
> > cause of the top-level error, so that way doesn't look right to me. My
> > way is used in some places, that way in other places.
>
> Well... I don't think you want a sqlite error to be returned as
> primary.

Ah, you mean if the reset returns an error, because I used SVN_ERR?
Yes, you're right. I was thinking of
svn_error_clear(svn_sqlite__reset(stmt)) as is done in
svn_wc__db_scan_addition(), whereas I copied the code from
svn_wc__db_base_get_dav_cache() which uses this undesirable construct.

> The PATH_NOT_FOUND is primary. Then, there is a secondary
> error around reset. Basically, it is just using create's CHILD param
> as a cheap composition of the errors (rather than
> svn_error_compose_create)

I see many of the functions compose the secondary error onto the primary
error's chain, in one way or another. But it doesn't make much sense to
me, theoretically. As I said, I think of the chain as being the
hierarchy of errors that lead to the primary error, so it seems wrong to
also put follow-up/clean-up errors in that chain.

Maybe each error should have both a "child" error chain that is
basically its "cause", and a list of "follow-up" error chains that are
the subsequent errors encountered in cleaning up after it. So an error
report is a tree rather than a chain. Pah. That's a thought for
another day... or never.

- Julian
Received on 2010-04-12 19:11:56 CEST

This is an archived mail posted to the Subversion Dev mailing list.