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

Re: svn commit: r1683126 - /subversion/trunk/subversion/libsvn_fs_x/cached_data.c

From: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Wed, 3 Jun 2015 13:39:34 +0300

On 3 June 2015 at 12:39, Philip Martin <philip.martin_at_wandisco.com> wrote:
> Bert Huijben <bert_at_qqmail.nl> writes:
>
>> Strangeā€¦ this should never be necessarily when *removing* something
>> from a hash (value =NULL).
>>
>>
>> I think you see some kind of other problem.
>
> I agree this bit is unnecessary, I have changed it back.
>
>> @@ -2494,7 +2494,9 @@ read_dir_entries(apr_array_header_t *ent
>> {
>> /* We must be in incremental mode */
>> assert(hash);
>> - apr_hash_set(hash, entry.key, entry.keylen, NULL);
>> + apr_hash_set(hash,
>> + apr_pstrmemdup(scratch_pool, entry.key, entry.keylen),
>> + entry.keylen, NULL);
>> continue;
>> }
>>
>
> This bit is the bug fix. It fixes some FAILs seen for fs-tests and
> valgrind identified memory reads after free. I wrote the fix in the
> wrong place first of all and then assumed I needed to fix it in both
> places.
>
>> @@ -2534,7 +2536,9 @@ read_dir_entries(apr_array_header_t *ent
>> /* In incremental mode, update the hash; otherwise, write to the
>> * final array. */
>> if (incremental)
>> - apr_hash_set(hash, entry.key, entry.keylen, dirent);
>> + apr_hash_set(hash,
>> + apr_pstrmemdup(scratch_pool, entry.key, entry.keylen),
>> + entry.keylen, dirent);
>> else
>> APR_ARRAY_PUSH(entries, svn_fs_x__dirent_t *) = dirent;
>> }
>
FWIW libsvn_fsfs uses dirent->name as key since it's already copied to
result_pool:
[[[
      /* In incremental mode, update the hash; otherwise, write to the
       * final array. Be sure to use hash keys that survive this iteration.
       */
      if (incremental)
        apr_hash_set(hash, dirent->name, entry.keylen, dirent);
      else
        APR_ARRAY_PUSH(entries, svn_fs_dirent_t *) = dirent;
]]]

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com
Received on 2015-06-03 12:40:55 CEST

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