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

Re: svn commit: r35823 - branches/explore-wc/subversion/libsvn_wc

From: Greg Stein <gstein_at_gmail.com>
Date: Sat, 14 Feb 2009 11:56:25 +0100

On Sat, Feb 14, 2009 at 06:04, Hyrum K. Wright
<hyrum_wright_at_mail.utexas.edu> wrote:
> On Feb 13, 2009, at 8:38 PM, Greg Stein wrote:
>> On Wed, Feb 11, 2009 at 22:57, Hyrum K. Wright <hyrum_at_hyrumwright.org>
>> wrote:
>>> ...
>>> +++ branches/explore-wc/subversion/libsvn_wc/entries.c Wed Feb 11
>>> 13:57:45 2009 (r35823)
>>> ...
>>> @@ -1138,6 +1147,20 @@ fetch_actual_nodes(apr_hash_t **nodes,
>>> }
>>>
>>> static svn_error_t *
>>> +fetch_wc_id(apr_int64_t *wc_id, svn_sqlite__db_t *wc_db)
>>> +{
>>> + svn_sqlite__stmt_t *stmt;
>>> + svn_boolean_t have_row;
>>> +
>>> + SVN_ERR(svn_sqlite__get_statement(&stmt, wc_db,
>>> STMT_SELECT_WCROOT_NULL));
>>> + SVN_ERR(svn_sqlite__step(&have_row, stmt));
>>> + if (!have_row)
>>> + return svn_error_create(SVN_ERR_WC_DB_ERROR, NULL, _("No WC table
>>> entry"));
>>
>> Simplify: use svn_sqlite__step_row(). And in lots of other places!
>>
>> And its companion svn_sqlite__step_done().
>
> Maybe I'm obtuse, or maybe it's just late, but the use of those APIs isn't
> readily intuitive. It appears that I need to know a priori how many rows
> there are to fetch, which isn't the case here.

Feel free to change those two APIs into something that makes more
sense (just checked; nothing uses them). In the above case, you *know*
that the step should return a row, so step_row() is Goodness. Shoot...
if a row isn't returned, you raise an error, which is *exactly* what
step_row() does. Just wrapped up into a simple API.

For DTML statements, no rows are expected, so step_done() makes sense.
That said... we already have an __insert() call, so maybe step_done is
bogus, in favor of __delete() and various __create() entry points. But
I'd say keep step_done -- we have the __insert() in order to return
the rowid. That isn't the case for other DTML statements.

> Oh, and svn_sqlite__step_done() finalizes the statement, and we'd rather
> have it reset instead.

No, it doesn't.

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1157193
Received on 2009-02-14 11:56:47 CET

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