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

Re: svn commit: r11927 - in branches/locking/subversion: include libsvn_fs_base libsvn_fs_base/bdb

From: Ben Collins-Sussman <sussman_at_collab.net>
Date: 2004-11-22 20:03:25 CET

On Nov 20, 2004, at 8:40 PM, Branko Čibej wrote:
>
>> + /* Get the first matching key that is either equal or greater
>> + * than the one passed in, by passing in the DB_RANGE_SET flag.
>> + */
>> + db_err = cursor->c_get(cursor, &key, svn_fs_base__result_dbt
>> (&value),
>> + DB_SET_RANGE);
>> +
>> + /* As long as the prefix of the returned KEY matches LOOKUP_PATH +
>> * we know it is either LOOKUP_PATH or a decendant thereof.
>> + */
>> + while (! db_err && strncmp(lookup_path, key.data, key.size) != 0)
>>
> I'm trying to grok this... say you looked up "foo/bar/", but got back
> "foo/bar/baz". If I read this correctly, you're doing a
> strncmp("foo/bar/", "foo/bar/baz", strlen("foo/bar/baz"))... haven't
> you got your lengths switced around? Also, "while prefix matches"
> implies "strncmp(...) == 0", not "strncmp(...) != 0).

Whoa. You are *so* right.

Sander wrote this stuff, and I should have reviewed it!

>
>> + /* Figure out the node_kind of this child path. */
>> + child_kind =
>> + (child_path[key.size - 1] == '/') ? svn_node_dir :
>> svn_node_file;
>> +
>> + /* If the child_path has a trailing '/', we need to remove it,
>> + to stay compatible with the rest of the fs library. */
>> + if (child_path[key.size - 1] == '/')
>> + child_path[key.size - 1] = '\0';
>>
> Hm. I thought we're not doing directory locks yet. Which means you
> can't get a path with a trailing slask from the tokens table.

Actually, we're ready for directory locks, in terms of our two locking
tables. The higher-level functions (svn_fs_lock) are simply preventing
us from ever locking a directory. But the locking table code takes
'kind' arguments, and knows to append/remove trailing slashes if kind
== svn_node_dir. We're preparing for the future, I guess. That's how
Sander wanted it.

>
>> + /* Make sure the token points to an existing, non-expired lock,
>> + by doing a lookup in the `locks' table. */
>> + err = svn_fs_bdb__lock_get (&lock, fs, lock_token, trail);
>> + if (err && ((err->apr_err == SVN_ERR_FS_LOCK_EXPIRED)
>> + || (err->apr_err == SVN_ERR_FS_BAD_LOCK_TOKEN)))
>> + {
>> + svn_error_clear (err);
>> +
>> + /* If `locks' doesn't have the lock, then we should lose it
>> + from `lock-tokens' table as well, then skip to the next
>> + matching path-key. */
>> + SVN_ERR (svn_fs_bdb__lock_token_delete (fs, child_path,
>> + child_kind,
>> trail));
>>
> Potential error leak again.
>

What about the svn_error_clear() right there?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Nov 22 20:05:42 2004

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.