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

Re: svn commit: r35160 - trunk/subversion/libsvn_client

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Mon, 12 Jan 2009 15:51:08 +0000

On Sun, 2009-01-11 at 14:25 -0800, Bert Huijben wrote:
> Author: rhuijben
> Date: Sun Jan 11 14:25:28 2009
> New Revision: 35160
>
> Log:
> Resolve 3 size truncation warnings.

Hold on a sec... This doesn't indicate that you made two opposite kinds
of change:

> * subversion/libsvn_client/info.c
> (build_info_from_dirent, build_info_from_entry):
> Truncate value to a maximum of 4GB to suppress warning.

Here you added a type cast to suppress the warning that we're not
reporting large files properly. The warning provided a useful service to
us, pointing out a problem. Let's not hide it.

If we aren't ready to upgrade the reporting structure to handle large
sizes, at least we should set the reported size to some value that looks
(at least to a human) like a "can't represent the value" marker, such as
(-1) or the maximum value of the type.

> * subversion/libsvn_client/merge.c
> (update_wc_mergeinfo): Use size_t instead of int for strlen variable.

... whereas here you fixed the code to be correct, which is great.

- Julian

> Modified: trunk/subversion/libsvn_client/info.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/info.c?pathrev=35160&r1=35159&r2=35160
> ==============================================================================
> --- trunk/subversion/libsvn_client/info.c Sun Jan 11 13:05:26 2009 (r35159)
> +++ trunk/subversion/libsvn_client/info.c Sun Jan 11 14:25:28 2009 (r35160)
> @@ -57,7 +57,9 @@ build_info_from_dirent(svn_info_t **info
> tmpinfo->lock = lock;
> tmpinfo->depth = svn_depth_unknown;
> tmpinfo->working_size = SVN_INFO_SIZE_UNKNOWN;
> - tmpinfo->size = dirent->size;
> + /* ### The size overflows on files > 4 GB.
> + We should add a new field of apr_off_t if we need the full value */
> + tmpinfo->size = (apr_size_t)dirent->size;
> tmpinfo->tree_conflict = NULL;
>
> *info = tmpinfo;
> @@ -99,7 +101,10 @@ build_info_from_entry(svn_info_t **info,
> tmpinfo->conflict_wrk = entry->conflict_wrk;
> tmpinfo->prejfile = entry->prejfile;
> tmpinfo->changelist = entry->changelist;
> - tmpinfo->working_size = entry->working_size;
> +
> + /* ### The working_size overflows on files > 4 GB.
> + We should add a new field of apr_off_t if we need the full value */
> + tmpinfo->working_size = (apr_size_t)entry->working_size;
> tmpinfo->size = SVN_INFO_SIZE_UNKNOWN;
>
> /* lock stuff */
>
> Modified: trunk/subversion/libsvn_client/merge.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_client/merge.c?pathrev=35160&r1=35159&r2=35160
> ==============================================================================
> --- trunk/subversion/libsvn_client/merge.c Sun Jan 11 13:05:26 2009 (r35159)
> +++ trunk/subversion/libsvn_client/merge.c Sun Jan 11 14:25:28 2009 (r35160)
> @@ -3381,7 +3381,7 @@ update_wc_mergeinfo(const char *target_w
> void *value;
> const char *path;
> apr_array_header_t *ranges, *rangelist;
> - int len;
> + size_t len;
> svn_error_t *err;
>
> svn_pool_clear(subpool);

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1019421
Received on 2009-01-12 16:51:29 CET

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