You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2001/05/09 22:17:33 UTC
Purify is your friend.
(Strictly speaking, this is an APR problem. But since it blew up in in
fs-test, and the culprit happens to be GregS, I thought y'all might like
to know about it. :-) )
Here's the deal:
apr_strings.h says:
/**
* duplicate the first n characters of a string into memory allocated
* out of a pool; the new string will be '\0'-terminated
* @param p The pool to allocate out of
* @param s The string to duplicate
* @param n The number of characters to duplicate
* @return The new string
* @deffunc char *apr_pstrndup(apr_pool_t *p, const char *s, apr_size_t n)
*/
cvs log apr_strings.c says:
----------------------------
revision 1.10
date: 2001/02/12 02:55:52; author: gstein; state: Exp; lines: +7 -8
*) rename apr_memdup() to apr_pmemdup()
*) optimize the allocation size in apr_pstrndup()
Purify says:
[E] ABR: Array bounds read in strlen {1 occurrence}
Reading 2 bytes from 0x0292e038 (1 byte at 0x0292e039 illegal)
Address 0x0292e038 is argument #1 of strlen
Address 0x0292e038 is at the beginning of a 1 byte block
Address 0x0292e038 points to a malloc'd block in heap 0x01c10000
Thread ID: 0x51c
Error location
strlen [MSVCRTD.dll]
apr_pstrndup [apr_strings.c:91]
if (s == NULL) {
return NULL;
}
=> len = strlen(s);
if (len < n)
n = len;
res = apr_palloc(a, n + 1);
allocate_txn_id [txn-table.c:133]
svn_fs__track_dbt (&value, trail->pool);
/* That's the value we want to return. */
=> next_id_str = apr_pstrndup (trail->pool, value.data, value.size);
/* Try to parse the value. */
{
svn_fs__create_txn [txn-table.c:171]
Now, you might say that the doc says nothing about apr_pstrndup not
needing a 0-terminated input. To which I reply that indeed, the doc says
nothing about needing a 0-terminated input.
Greg, will you fix it, or shall I? :-)
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Purify is your friend.
Posted by Karl Fogel <kf...@collab.net>.
Branko Čibej <br...@xbc.nu> writes:
> Done. Works. No more ABRs in fs_test.
Thanks for all the Purify work, Branko.
> Now, could somebody please test this on a box that doesn't have memchr. I've
> gone and provided a (tested!) memchr implementation in APR for just this
> purpose, but I don't know of any such boxen.
I have an old box in my closet, seems to have vacuum tubes, some
switches, British Office of Home Defense markings all over it, never
been sure what it does...
:-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Purify is your friend.
Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:
>On Thu, May 10, 2001 at 08:41:13AM +0200, brane@xbc.nu wrote:
>
>>...
>>The thing is, one would expect apr_pstrndup to behave like the standard strn*
>>functions, which also operate on "string"s, but guarantee not to touch anything
>>past the "n". (It doesn't /have/ to, of course.)
>>
>>Would it be O.K. if I changed the length check to use a memchr of '\0' instead
>>of strlen?
>>
>
>Seems like a fine solution!
>
Done. Works. No more ABRs in fs_test.
Now, could somebody please test this on a box that doesn't have memchr.
I've gone and provided a (tested!) memchr implementation in APR for just
this purpose, but I don't know of any such boxen.
--
Brane �ibej
home: <br...@xbc.nu> http://www.xbc.nu/brane/
work: <br...@hermes.si> http://www.hermes-softlab.com/
ACM: <br...@acm.org> http://www.acm.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Purify is your friend.
Posted by Greg Stein <gs...@lyra.org>.
On Thu, May 10, 2001 at 08:41:13AM +0200, brane@xbc.nu wrote:
>...
> The thing is, one would expect apr_pstrndup to behave like the standard strn*
> functions, which also operate on "string"s, but guarantee not to touch anything
> past the "n". (It doesn't /have/ to, of course.)
>
> Would it be O.K. if I changed the length check to use a memchr of '\0' instead
> of strlen?
Seems like a fine solution!
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: Purify is your friend.
Posted by br...@xbc.nu.
Quoting Greg Stein <gs...@lyra.org>:
> On Thu, May 10, 2001 at 12:17:33AM +0200, Branko �ibej wrote:
> > (Strictly speaking, this is an APR problem. But since it blew up in in
>
> > fs-test, and the culprit happens to be GregS, I thought y'all might
> like
> > to know about it. :-) )
>
> Okay... where'd it blow up, and will you be checking in a fix?
:-)
It dies in the FS when we're copying a key from a DBT to a pool. The data coming
from Berkeley DB is not 0-terminated, of course.
>
> > Here's the deal:
> >
> > apr_strings.h says:
> >
> > /**
> > * duplicate the first n characters of a string into memory allocated
>
> "of a string" typically implies something that is null-terminated.
>
> But if you want to clarify the doc in apr_strings.h, then please go
> ahead (I believe you have commit privs to APR, right?)
The thing is, one would expect apr_pstrndup to behave like the standard strn*
functions, which also operate on "string"s, but guarantee not to touch anything
past the "n". (It doesn't /have/ to, of course.)
Would it be O.K. if I changed the length check to use a memchr of '\0' instead
of strlen? The thing is, apart from this little problem, apr_pstrndup does
exactly what we need. We cant use apr_pmemdup, obviously, because we have to
end up with a 0-terminated string.
I could just go and clarify the doc, and change the FS, but it seems such a
waste ...
Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Purify is your friend.
Posted by Greg Stein <gs...@lyra.org>.
On Thu, May 10, 2001 at 12:17:33AM +0200, Branko �ibej wrote:
> (Strictly speaking, this is an APR problem. But since it blew up in in
> fs-test, and the culprit happens to be GregS, I thought y'all might like
> to know about it. :-) )
Okay... where'd it blow up, and will you be checking in a fix?
> Here's the deal:
>
> apr_strings.h says:
>
> /**
> * duplicate the first n characters of a string into memory allocated
"of a string" typically implies something that is null-terminated.
But if you want to clarify the doc in apr_strings.h, then please go ahead (I
believe you have commit privs to APR, right?)
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