[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 <julianfoad_at_btopenworld.com>
Date: Mon, 12 Apr 2010 17:35:54 +0100

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:
> >...
> > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Apr 12 15:19:23 2010
> > @@ -7254,13 +7254,17 @@ svn_wc__db_get_pristine_md5(const svn_ch
> > SVN_ERR(svn_sqlite__step(&have_row, stmt));
> > if (!have_row)
> > {
> > - *md5_checksum = NULL; /* ### that's not what we want. Report an error
> > - instead. */
> > - return svn_error_return(svn_sqlite__reset(stmt));
> > + *md5_checksum = NULL;
>
> There is no need to worry about the OUT params if you throw an error.

Oops, I missed that.

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

> I don't think that error code is appropriate, however. I would suggest
> either SVN_ERR_WC_DB_ERROR or creating a new code.

Ah, yes - didn't notice that.

Fixed in r933307.

Thanks.
- Julian
Received on 2010-04-12 18:36:30 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.