RE: svn commit: r1665853 - /subversion/trunk/subversion/libsvn_diff/diff_file.c

r1665853 is not in STATUS as far as I can tell. (I usually don’t nominate warning fixes).

Otherwise I would have started by libsvn_fsx:

[[

1> verify.c

1>..\..\..\subversion\libsvn_fs_x\verify.c(443): warning C4244: 'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1> util.c

1> tree.c

1> transaction.c

1>..\..\..\subversion\libsvn_fs_x\transaction.c(3297): warning C4244: '=' : conversion from 'apr_size_t' to 'unsigned char', possible loss of data

1> temp_serializer.c

1>..\..\..\subversion\libsvn_fs_x\temp_serializer.c(628): warning C4090: 'function' : different 'const' qualifiers

1>..\..\..\subversion\libsvn_fs_x\temp_serializer.c(631): warning C4090: 'function' : different 'const' qualifiers

1>..\..\..\subversion\libsvn_fs_x\temp_serializer.c(844): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\temp_serializer.c(946): warning C4018: '>' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\temp_serializer.c(1327): warning C4090: 'function' : different 'const' qualifiers

1> string_table.c

1>..\..\..\subversion\libsvn_fs_x\string_table.c(184): warning C4244: 'return' : conversion from 'int' to 'apr_uint16_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\string_table.c(222): warning C4244: 'return' : conversion from 'int' to 'apr_uint16_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\string_table.c(262): warning C4244: 'return' : conversion from 'int' to 'apr_uint16_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\string_table.c(410): warning C4244: '=' : conversion from 'const int' to 'apr_uint16_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\string_table.c(720): warning C4244: '=' : conversion from 'apr_uint64_t' to 'apr_size_t', possible loss of data

1> revprops.c

1>..\..\..\subversion\libsvn_fs_x\revprops.c(907): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\revprops.c(908): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\revprops.c(920): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1> rev_file.c

1> reps.c

1>..\..\..\subversion\libsvn_fs_x\reps.c(475): warning C4018: '>=' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\reps.c(537): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\reps.c(542): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\reps.c(614): warning C4018: '>=' : signed/unsigned mismatch

1> rep-cache.c

1> recovery.c

1> pack.c

1>..\..\..\subversion\libsvn_fs_x\pack.c(421): warning C4244: 'function' : conversion from 'apr_int64_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(458): warning C4244: 'initializing' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(520): warning C4244: 'function' : conversion from 'svn_fs_x__change_set_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(832): warning C4244: 'return' : conversion from 'apr_int64_t' to 'apr_ssize_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1002): warning C4018: '>' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\pack.c(1158): warning C4244: '+=' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1287): warning C4244: 'function' : conversion from 'svn_filesize_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1289): warning C4244: '=' : conversion from 'svn_filesize_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1297): warning C4389: '==' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\pack.c(1298): warning C4244: '-=' : conversion from 'apr_off_t' to 'apr_ssize_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1334): warning C4244: '+=' : conversion from 'apr_off_t' to 'apr_ssize_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1567): warning C4244: 'initializing' : conversion from 'apr_off_t' to 'apr_ssize_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\pack.c(1613): warning C4389: '==' : signed/unsigned mismatch

1> noderevs.c

1> low_level.c

1> lock.c

1> index.c

1>..\..\..\subversion\libsvn_fs_x\index.c(1227): warning C4244: '-=' : conversion from 'apr_uint64_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\index.c(1491): warning C4244: 'function' : conversion from 'apr_uint64_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\index.c(1491): warning C4244: 'function' : conversion from 'apr_uint64_t' to 'size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\index.c(1525): warning C4389: '!=' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(1968): warning C4018: '>=' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(2064): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(2128): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(2298): warning C4244: '=' : conversion from 'apr_uint64_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\index.c(2531): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\index.c(2772): warning C4018: '<' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(3060): warning C4018: '>=' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(3084): warning C4018: '>' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\index.c(3698): warning C4018: '<' : signed/unsigned mismatch

1> id.c

1>..\..\..\subversion\libsvn_fs_x\id.c(87): warning C4146: unary minus operator applied to unsigned type, result still unsigned

1> hotcopy.c

1> fs_x.c

1> fs_id.c

1> Generating Code...

1>f:\svn-dev\dev\subversion\libsvn_fs_x\pack.c(2209): warning C4703: potentially uninitialized local pointer variable 'revprops_shard_path' used

1>f:\svn-dev\dev\subversion\libsvn_fs_x\rev_file.c(174): warning C4703: potentially uninitialized local pointer variable 'apr_file' used

1>f:\svn-dev\dev\subversion\libsvn_fs_x\tree.c(1088): warning C4703: potentially uninitialized local pointer variable 'child' used

1> Compiling...

1> fs.c

1> dag.c

1> changes.c

1> caching.c

1> cached_data.c

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(1489): warning C4244: 'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(1780): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(1822): warning C4244: 'function' : conversion from 'svn_filesize_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(2369): warning C4389: '==' : signed/unsigned mismatch

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(2593): warning C4244: 'initializing' : conversion from 'svn_filesize_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(2903): warning C4244: 'function' : conversion from 'apr_int64_t' to 'svn_revnum_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(2930): warning C4244: 'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1>..\..\..\subversion\libsvn_fs_x\cached_data.c(2931): warning C4244: '=' : conversion from 'apr_off_t' to 'apr_size_t', possible loss of data

1> Generating Code...

1>f:\svn-dev\dev\subversion\libsvn_fs_x\cached_data.c(754): warning C4703: potentially uninitialized local pointer variable 'rh' used

1> libsvn_fs_x.vcxproj -> F:\svn-dev\dev\Debug\subversion\libsvn_fs_x\libsvn_fs_x-1.lib

]]

                Bert

From: Branko Čibej [mailto:brane_at_wandisco.com]
Sent: donderdag 12 maart 2015 09:27
To: Bert Huijben; 'Subversion Development'
Subject: Re: svn commit: r1665853 - /subversion/trunk/subversion/libsvn_diff/diff_file.c

On 12.03.2015 08:49, Bert Huijben wrote:

This resolves several signed compared to unsigned warnings on Windows.

It is nice to fix warnings for some favorite compiler, but there are still > 200 warnings left on Windows, especially on cases that assume int and size_t variants have identical length, which doesn’t hold on Win64.

Our policy is to try to compile without warnings in maintainer mode on most platforms. I realize that Windows tends to be an exception (although, to be fair, many of this class of warnings in the Windows build are false positives).

But the point is that your change added a new warning about a narrowing assignment; in other words, there must be a better way to both resolved the signed/unsigned warnings you see, and the narrowing conversion warning that I see; which, by the way, *is* important on both Win32 and Win64, since we're now assigning an unsigned value to a signed one, even if it's of the same size, so overflow could happen in theory.

For example, if you change the type of context_size in svn_diff3__file_output_baton_t, you probably have to change it in context_saver_t as well. Both structs are local to diff_file.c (which makes me wonder while svn_diff3__file_output_baton_t has that svn_diff3__ prefix in the first place). Assuming that using an unsigned value there is even correct.

I noticed the new warning while I was reviewing r1665853 in STATUS for 1.9.x, by the way.

-- Brane

From: Branko Čibej <mailto:brane_at_wandisco.com>

Sent: ‎Thursday‎, ‎March‎ ‎12‎, ‎2015 ‎4‎:‎59‎ ‎AM
To: 'Subversion Development' <mailto:dev_at_subversion.apache.org>

On 11.03.2015 13:02, rhuijben_at_apache.org <mailto:rhuijben_at_apache.org> wrote:
> Author: rhuijben
> Date: Wed Mar 11 12:02:50 2015
> New Revision: 1665853
>
> URL: http://svn.apache.org/r1665853
> Log:
> * subversion/libsvn_diff/diff_file.c
> (svn_diff3__file_output_baton_t): Use apr_size_t to avoid some warnings,
> on usages of this variable.
>
> Modified:
> subversion/trunk/subversion/libsvn_diff/diff_file.c
>
> Modified: subversion/trunk/subversion/libsvn_diff/diff_file.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/diff_file.c?rev=1665853 <http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/diff_file.c?rev=1665853&r1=1665852&r2=1665853&view=diff> &r1=1665852&r2=1665853&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_diff/diff_file.c (original)
> +++ subversion/trunk/subversion/libsvn_diff/diff_file.c Wed Mar 11 12:02:50 2015
> @@ -1998,7 +1998,7 @@ typedef struct svn_diff3__file_output_ba
> const char *marker_eol;
>
> svn_diff_conflict_display_style_t conflict_style;
> - int context_size;
> + apr_size_t context_size;
>
> /* cancel support */
> svn_cancel_func_t cancel_func;

I don't know what kind of warnings you're avoiding here, but I saw none
in maintainer mode before this commit, and now I'm getting this:

subversion/libsvn_diff/diff_file.c:2051:27: warning: implicit conversion loses integer precision:
      'apr_size_t' (aka 'unsigned long') to 'int' [-Wshorten-64-to-32]
  cs->context_size = fob->context_size;
                   ~ ~~~~~^~~~~~~~~~~~
1 warning generated.

-- Brane

 
Received on 2015-03-12 09:54:22 CET