[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: Tue, 17 Feb 2009 22:55:17 +0100

On Tue, Feb 17, 2009 at 22:34, Hyrum K. Wright
<hyrum_wright_at_mail.utexas.edu> wrote:
>
> On Feb 14, 2009, at 4:56 AM, Greg Stein wrote:
>
>> 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.
>
> Ah gotcha. I'll take a look at where they can be used.
>
>>> Oh, and svn_sqlite__step_done() finalizes the statement, and we'd rather
>>> have it reset instead.
>>
>> No, it doesn't.
>
> In which case, the docs are lying:
>
> /* Steps the given statement; raises an SVN error (and finalizes the
> statement) if it doesn't return SQLITE_DONE. */
> svn_error_t *
> svn_sqlite__step_done(svn_sqlite__stmt_t *stmt);

Doc strings?!

Okay. Thanks, Grandma.

Cheers,
-Greg "Reads the Code" Stein

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1181350
Received on 2009-02-17 22:55:39 CET

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