[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 03:38:14 +0100

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

> + *wc_id = svn_sqlite__column_int(stmt, 0);

I just looked at that SQL statement, and it tests local_abspath for
being NULL. But local_abspath is declared as NOT NULL.

Further, looking at the INSERT for that row, the SQL has a value to
insert, but we bind nothing to it.

I think the right answer is to change the schema, and add a comment
that NULL means "implied by the location this database in the
filesystem. e.g. for /some/path/.svn/wc.db, the local_abspath is
implied as '/some/path'"

>...
> + /* Also remove from the sqlite database. */
> + /* Open the wc.db sqlite database. */
> + SVN_ERR(svn_sqlite__open(&wc_db, db_path(parent_dir, scratch_pool),
> + svn_sqlite__mode_readwrite, statements,
> + SVN_WC__VERSION, upgrade_sql,
> + scratch_pool, scratch_pool));

Note that we "could" cache open databases in the adm_access stuff. But
I believe that is going away entirely (for API consistency, we'll have
a placebo adm_access structure). Further, all sqlite db handles will
be managed (and cached/reused/etc) under the wc_db.h covers.

>...

Cheers,
-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1154870
Received on 2009-02-14 03:38:29 CET

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