On Tue, 18 Jul 2006 01:35:56 +0530, Daniel Rall <dlr@collab.net> wrote:
[snip]
>
> This is clearly redundant. The following snippet was all added in 20091:
>
> ...
> + }
> + apr_hash_set(*result, mergedfrom, APR_HASH_KEY_STRING,
> pathranges);
> +
> + if (sqlite_result != SQLITE_DONE)
> + return svn_error_create(SVN_ERR_FS_SQLITE_ERROR, NULL,
> + sqlite3_errmsg(db));
> +
> + apr_hash_set(*result, mergedfrom, APR_HASH_KEY_STRING,
> + pathranges);.
>
> However, why remove the second occurrence instead of the first? If we
> detect an error, wouldn't we want to fail-fast instead of setting a
> possibly bogus value in the hash?
Because, the while loop keeps building valid apr_array_header_t of
svn_merge_range_t s, which never get set into the hash unless control
comes out of the while or when control enters either the following if:
if (lastmergedfrom && strcmp(mergedfrom, lastmergedfrom) != 0)
So, it is possible that while the last row has not returned SQLITE_DONE,
but we already have some data that we have collected from the db, that we
might as well put into the hash, before erroring out. Am not very
confident of this, but I think this might help us debug which exact row
was the last before the SQLITE error.
Regards,
Madan.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jul 18 08:55:14 2006