[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: Greg Stein <gstein_at_gmail.com>
Date: Mon, 12 Apr 2010 13:21:28 -0400

On Mon, Apr 12, 2010 at 13:11, Julian Foad <julian.foad_at_wandisco.com> wrote:
> 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(),

Ah. Yeah, just clearing it would be fine in this case, tho I tend to
like keeping them. When I wrote that line in scan_addition, I may not
have been up-to-date on the composition stuffs. Who knows.

> whereas I copied the code from
> svn_wc__db_base_get_dav_cache() which uses this undesirable construct.

Yah. Ugh.

>>  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.

Sure. *shrug*

>...

Cheers,
-g
Received on 2010-04-12 19:21:58 CEST

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