[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fix possible overflow with very large files in libsvn_delta

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2004-11-02 13:40:10 CET

Norbert Unterberg wrote:
> This is my first contribution so please react friendly on my possible
> ignorance to your contribution rules :-)

The way you have written your patch and the log message all looks good. There
is just one thing that is awkward: it is attached to this e-mail with a MIME
type of "application/octet-stream" which means we have to save it to disk and
then open it in a separate editor in order to see it. Please try to attach it
as type "text/plain" so that many of us will see it displayed in-line in our
mail programs. People will be more likely to review it if you do that.

> When building TortoiseSVN, I noticed some compiler warnings when building
> some of the SVN libraries. I looked closer at them and found a possible
> problem in libsvn_delta. There is some piece of code that stores stream
> offsets in a apr_size_t variable. This might fail on systems that support
> very large files (64 bit file offsets stored in a 32 bit integer). The fix
> was easy and I just replaced apr_size_t with svn_file_size at two places.

That fix looks good to me. I assume you have run "make check" (Have you?) but
have not tested that it fixes the handling of very large files. It should be
fairly easy to test, for someone who has enough disk space for two or three
copies of a just-over-4GB file. But maybe it is not necessary to test it in
that way, because even if such a large file fails for other reasons, this fix
appears to be correct in itself.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Nov 2 13:40:43 2004

This is an archived mail posted to the Subversion Dev mailing list.