You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Philip Martin <ph...@codematters.co.uk> on 2002/05/15 02:18:45 UTC
Another valgrind warning, deltify_by_id()
Hello
I'm still playing with valgrind, trying to determine if the warnings
it gives are genuine. Consider this, it warns about the following
code in deltify_by_id()
apr_size_t len = svn_fs__id_length (target_id);
...
tmp_id = apr_palloc (trail->pool, sizeof (*tmp_id));
tmp_id->digits = apr_pmemdup (trail->pool, target_id->digits,
(len + 3) * sizeof (target_id->digits[0]));
tmp_id->digits[len] = 1;
tmp_id->digits[len + 1] = 1;
tmp_id->digits[len + 2] = -1;
claiming that the memcpy within apr_pmemdup is reading from beyond the
end of allocated memory
This warning looks correct to me: it appears that the code is
allocating 2 digits more in tmp_id than exist in target_id since
svn_fs__id_length() doesn't count the terminating -1. Thus the
apr_pmemdup will try to copy more digits from target_id than actually
exist.
I believe the code should be something like
tmp_id->digits = apr_palloc (trail->pool,
(len + 3) * sizeof (target_id->digits[0]));
memcpy (tmp_id->digits, target_id->digits,
len * sizeof (target_id->digits[0]));
tmp_id->digits[len] = 1;
tmp_id->digits[len + 1] = 1;
tmp_id->digits[len + 2] = -1;
I'm getting to be quite impressed by valgrind!
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Another valgrind warning, deltify_by_id()
Posted by Greg Stein <gs...@lyra.org>.
On Wed, May 15, 2002 at 03:18:45AM +0100, Philip Martin wrote:
>...
> tmp_id->digits = apr_pmemdup (trail->pool, target_id->digits,
> (len + 3) * sizeof (target_id->digits[0]));
>...
> claiming that the memcpy within apr_pmemdup is reading from beyond the
> end of allocated memory
It's right, as you suspected :-)
> This warning looks correct to me: it appears that the code is
> allocating 2 digits more in tmp_id than exist in target_id since
> svn_fs__id_length() doesn't count the terminating -1. Thus the
> apr_pmemdup will try to copy more digits from target_id than actually
> exist.
Yup.
> I believe the code should be something like
>
> tmp_id->digits = apr_palloc (trail->pool,
> (len + 3) * sizeof (target_id->digits[0]));
> memcpy (tmp_id->digits, target_id->digits,
> len * sizeof (target_id->digits[0]));
> tmp_id->digits[len] = 1;
> tmp_id->digits[len + 1] = 1;
> tmp_id->digits[len + 2] = -1;
>
> I'm getting to be quite impressed by valgrind!
Agreed. +1
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Another valgrind warning, deltify_by_id()
Posted by Daniel Berlin <db...@dberlin.org>.
On 15 May 2002, Philip Martin wrote:
> Hello
>
> I'm still playing with valgrind, trying to determine if the warnings
> it gives are genuine. Consider this, it warns about the following
> code in deltify_by_id()
>
> apr_size_t len = svn_fs__id_length (target_id);
> ...
> tmp_id = apr_palloc (trail->pool, sizeof (*tmp_id));
> tmp_id->digits = apr_pmemdup (trail->pool, target_id->digits,
> (len + 3) * sizeof (target_id->digits[0]));
> tmp_id->digits[len] = 1;
> tmp_id->digits[len + 1] = 1;
> tmp_id->digits[len + 2] = -1;
>
> claiming that the memcpy within apr_pmemdup is reading from beyond the
> end of allocated memory
>
> This warning looks correct to me: it appears that the code is
> allocating 2 digits more in tmp_id than exist in target_id since
> svn_fs__id_length() doesn't count the terminating -1. Thus the
> apr_pmemdup will try to copy more digits from target_id than actually
> exist.
>
> I believe the code should be something like
>
> tmp_id->digits = apr_palloc (trail->pool,
> (len + 3) * sizeof (target_id->digits[0]));
> memcpy (tmp_id->digits, target_id->digits,
> len * sizeof (target_id->digits[0]));
> tmp_id->digits[len] = 1;
> tmp_id->digits[len + 1] = 1;
> tmp_id->digits[len + 2] = -1;
>
> I'm getting to be quite impressed by valgrind!
--gdb-attach=yes is your friend.
It's quite a nice piece of work.
It actually is a x86 protected mode emulator.
Okay, an x86-to-x86 JITer.
It does it's own register allocation an everything on the internal form.
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org