On 3 June 2015 at 12:39, Philip Martin <philip.mar...@wandisco.com> wrote:
> Bert Huijben <b...@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
