You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by cm...@collab.net on 2003/02/10 05:12:31 UTC
Oops.
I've noticed that our new UUIDs include a NULL character as part of
the stored db data. Is this intentional? I don't particularly like
it.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Oops.
Posted by cm...@collab.net.
cmpilato@collab.net writes:
> I've noticed that our new UUIDs include a NULL character as part of
> the stored db data. Is this intentional? I don't particularly like
> it.
Well, I've fixed it now (revision 4813). As far as I can tell (by
some hand-testing), this should not affect the usability of databases
that already have the 'uuids' table in place.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Oops.
Posted by Philip Martin <ph...@codematters.co.uk>.
mark benedetto king <mb...@boredom.org> writes:
> I see that you've done
>
> *uuid = apr_pstrmemdup(trail->pool, value.data, value.size + 1);
>
> It is my impression that BDB makes no guarantees about what the value
> of the byte at value.data[value.size] is. It might not be NUL, and
> I'm pretty sure I've seen it as ' '.
>
> Anyway, because of this I went and actually *looked* at apr_pstrmemdup(),
> and I'll bet the right thing to do is change this to
>
> *uuid = apr_pstrmemdup(trail->pool, value.data, value.size);
>
> because apr_pstrmemdup() automatically allocates N+1 bytes and NUL-terminates.
I'm using Subversion r4816
$ svnadmin create repo
$ valgrind -q svnadmin dump repo > zz
==30155== Invalid read of size 1
==30155== at 0x40048090: memcpy (/home/pm/sw/valgrind-1.0.4/vg_clientfuncs.c:496)
==30155== by 0x40390947: apr_pstrmemdup (apr_strings.c:147)
==30155== by 0x4026848B: svn_fs__bdb_get_uuid (../svn/subversion/libsvn_fs/bdb/uuids-table.c:109)
==30155== by 0x4027BAC4: txn_body_get_uuid (../svn/subversion/libsvn_fs/uuid.c:38)
==30155== Address 0x430D430C is 0 bytes after a block of size 36 alloc'd
==30155== at 0x400476BB: malloc (/home/pm/sw/valgrind-1.0.4/vg_clientfuncs.c:100)
==30155== by 0x403422EE: __os_malloc_4000 (in /usr/lib/libdb-4.0.so)
==30155== by 0x4034236F: __os_realloc_4000 (in /usr/lib/libdb-4.0.so)
==30155== by 0x40317462: __db_retcopy_4000 (in /usr/lib/libdb-4.0.so)
* Dumped revision 0.
Which implies two things: first the +1 is indeed wrong, and second you
are not setting the correct flags to use the local stack based buffer
in svn_fs__bdb_get_uuid. You need to specify DB_DBT_USERMEM and set
ulen. Why use stack memory at all? If you use pool memory directly,
then you don't need to copy it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Oops.
Posted by mark benedetto king <mb...@boredom.org>.
On Sun, Feb 09, 2003 at 11:12:31PM -0600, cmpilato@collab.net wrote:
> I've noticed that our new UUIDs include a NULL character as part of
> the stored db data. Is this intentional? I don't particularly like
> it.
>
I had it without the null at first, but I thought adding it in
made the code a little cleaner because I didn't have to worry about
null-terminating the data coming *out* of the table.
I see that you've done
*uuid = apr_pstrmemdup(trail->pool, value.data, value.size + 1);
It is my impression that BDB makes no guarantees about what the value
of the byte at value.data[value.size] is. It might not be NUL, and
I'm pretty sure I've seen it as ' '.
Anyway, because of this I went and actually *looked* at apr_pstrmemdup(),
and I'll bet the right thing to do is change this to
*uuid = apr_pstrmemdup(trail->pool, value.data, value.size);
because apr_pstrmemdup() automatically allocates N+1 bytes and NUL-terminates.
Then we get to have our cake (not store the NULs) and eat it too (not
screw around with NUL termination).
--ben
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org