Re: a bug in svn_fs_base__unparse_representation_skel?

Mary Jane Lemur wrote:
> i'm looking in libsvn_fs_base/util/fs_skels.c, line 1038.
>
> the code seems to be duplicating the rep->checksum array in memory by
> calling svn_fs_base__mem_atom. i understand that it's passing
> rep->checksum as the 'data' argument, but for the 'length' argument
> its passing
>
> APR_MD5_DIGESTSIZE / sizeof (*(rep->checksum)
>
> isnt this wrong? if we're on a system where (unsigned char) is 2
> bytes, then dont we want to allocate a memory space thats twice as
> big? instead of dividing, shouldnt it be multiplying by the sizeof()?
> like
>
> APR_MD5_DIGESTSIZE * sizeof (*(rep->checksum)

Yes; or, at least, that's better than dividing by it.

> i'm guessing that this bug hasnt been noticed because every system so
> far has a 1-byte unsigned char, so we're dividing APR_MD5_DIGESTSIZE
> by 1, which works out ok. just lucky, mebbe?

Most of the code assumes a char is one byte. svn_fs_base__mem_atom() and
sizeof() both work in bytes. APR doesn't specify (in apr_md5.h) what the units
of APR_MD5_DIGESTSIZE are, but we treat it as if the units are bytes or chars
interchangeably. Therefore the fix is:

   APR_MD5_DIGESTSIZE

(not multiplied or divided by anything), just like everywhere else it is used
for "memcpy()" etc. An alternative would be:

   sizeof(rep->checksum)

but that wouldn't work if rep->checksum were to be changed to a pointer rather
than a fixed array.

Fixed in r17704 (along with some useless backslashes that I happened to notice).

Thank you for pointing it out.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Fri Dec 9 02:25:23 2005