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 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&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:27:39 CET