That makes sense, I've committed r1464413.
Ben Reser <ben_at_reser.org> writes:
> You should apply the later one. The check for the kind is redundant.
>
> Just before this check we call svn_fs_base__dag_file_checksum() and
> specify that we want tb->base_checksum->kind. Ultimately, this calls
> svn_fs_base__rep_contents_checksums() which just does a
> svn_checksum_dup on the checksum in the representation of the kind we
> asked for. So ultimately, that kind test in the if statement ends up
> testing that our functions are following their contract.
>
> On Wed, Apr 3, 2013 at 2:04 PM, Philip Martin
> <philip.martin_at_wandisco.com> wrote:
>> As part of http://subversion.tigris.org/issues/show_bug.cgi?id=4344 I
>> was playing with a BDB repository created by driving the svn_fs API
>> directly in repos-test.c:node_locations2. This creates a revision where
>> a file has no checksum. A subsequent commit that modifies the file can
>> SEGV:
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> [Switching to Thread 0x7ff8d49ab700 (LWP 4957)]
>> 0x00007ff8d6430320 in txn_body_apply_textdelta (baton=0x7ff8da368dc0,
>> trail=0x7ff8da368e60) at ../src/subversion/libsvn_fs_base/tree.c:3733
>> 3733 if (tb->base_checksum->kind == checksum->kind
>>
>> because svn_fs_base__dag_file_checksum returns a NULL checksum as the
>> documentation allows (repos-test itself doesn't SEGV because it
>> doesn't pass a base checksum, but if you interrupt the test after r2 and
>> then checkout/commit using a standard client the SEGV occurs).
>>
>> I can fix the SEGV using this patch:
>>
>> Index: ../src/subversion/libsvn_fs_base/tree.c
>> ===================================================================
>> --- ../src/subversion/libsvn_fs_base/tree.c (revision 1464080)
>> +++ ../src/subversion/libsvn_fs_base/tree.c (working copy)
>> @@ -3730,7 +3730,7 @@
>> we're calculating both SHA1 and MD5 checksums somewhere in
>> reps-strings.c. Could we keep them both around somehow so this
>> check could be more comprehensive? */
>> - if (tb->base_checksum->kind == checksum->kind
>> + if (checksum && tb->base_checksum->kind == checksum->kind
>> && !svn_checksum_match(tb->base_checksum, checksum))
>> return svn_checksum_mismatch_err(tb->base_checksum, checksum,
>> trail->pool,
>>
>> If I look at the FSFS code it doesn't do the kind comparison, it was
>> removed in r874326 but that's a revert which may have reverted more than
>> intended. The FSFS code does:
>>
>> if (!svn_checksum_match(tb->base_checksum, checksum))
>> return svn_checksum_mismatch_err(tb->base_checksum, checksum, pool,
>> _("Base checksum mismatch on '%s'"),
>> tb->path);
>>
>> so I can also fix the SEGV with:
>>
>> Index: ../src/subversion/libsvn_fs_base/tree.c
>> ===================================================================
>> --- ../src/subversion/libsvn_fs_base/tree.c (revision 1464080)
>> +++ ../src/subversion/libsvn_fs_base/tree.c (working copy)
>> @@ -3730,8 +3730,7 @@
>> we're calculating both SHA1 and MD5 checksums somewhere in
>> reps-strings.c. Could we keep them both around somehow so this
>> check could be more comprehensive? */
>> - if (tb->base_checksum->kind == checksum->kind
>> - && !svn_checksum_match(tb->base_checksum, checksum))
>> + if (!svn_checksum_match(tb->base_checksum, checksum))
>> return svn_checksum_mismatch_err(tb->base_checksum, checksum,
>> trail->pool,
>> _("Base checksum mismatch on '%s'"),
>>
>> Which patch should I apply? Should BDB and FSFS be the same?
>>
>> --
>> Certified & Supported Apache Subversion Downloads:
>> http://www.wandisco.com/subversion/download
>
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Received on 2013-04-04 12:12:03 CEST