Re: [PATCH]svn_fs_base__txn_proplist_in_trail never sets a null txnproplist(leverage on this fact).

Kamesh Jayachandran wrote:
> Can someone respond to this?

Kamesh,
I've gone ahead and filed this as issue 2637.

-Hyrum

> With regards
> Kamesh Jayachandran
> Kamesh Jayachandran wrote:
>> Madan U Sreenivasan wrote:
>>> On Fri, 06 Oct 2006 01:06:16 +0530, Kamesh Jayachandran
>>> <kamesh@collab.net> wrote:
>>>
>>>> Index: subversion/libsvn_fs_base/dag.c
>>>> ===================================================================
>>>> --- subversion/libsvn_fs_base/dag.c(revision 21673)
>>>> +++ subversion/libsvn_fs_base/dag.c(working copy)
>>>> @@ -1428,18 +1428,13 @@
>>>> /* Remove any temporary transaction properties initially created by
>>>> begin_txn(). */
>>>> SVN_ERR(svn_fs_base__txn_proplist_in_trail(&txnprops, txn_id,
>>>> trail));
>>>> - if (txnprops)
>>>> - {
>>>> - if (apr_hash_get(txnprops, SVN_FS_PROP_TXN_CHECK_OOD,
>>>> - APR_HASH_KEY_STRING))
>>>> - SVN_ERR(svn_fs_base__set_txn_prop
>>>> - (fs, txn_id, SVN_FS_PROP_TXN_CHECK_OOD, NULL,
>>>> trail, pool));
>>>> + if (apr_hash_get(txnprops, SVN_FS_PROP_TXN_CHECK_OOD,
>>>> APR_HASH_KEY_STRING))
>>>> + SVN_ERR(svn_fs_base__set_txn_prop
>>>> + (fs, txn_id, SVN_FS_PROP_TXN_CHECK_OOD, NULL, trail,
>>>> pool));
>>>> - if (apr_hash_get(txnprops, SVN_FS_PROP_TXN_CHECK_LOCKS,
>>>> - APR_HASH_KEY_STRING))
>>>> - SVN_ERR(svn_fs_base__set_txn_prop
>>>> - (fs, txn_id, SVN_FS_PROP_TXN_CHECK_LOCKS, NULL,
>>>> trail, pool));
>>>> - }
>>>
>>> Is there any specific reason you have removed the check for non-NULL
>>> of txnprops? iirc, apr_hash_get() could crash if txnprops is NULL.
>>>
>> Patch is all about the fact txnprops will never be null(see the
>> implementation of 'svn_fs_base__txn_proplist_in_trail', so let us
>> leverage on this fact).