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

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;
> }

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*