On 12.03.2015 09:27, Branko Čibej wrote: > 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.
Looking at the code some more, it turns out that context_size is an int in lots of places: svn_diff_options_t, svn_diff__file_output_baton_t, context_saver_t, many function parameters in that file, etc. The code almost invariably expects a signed value. In other words, changing the type only in svn_diff3__file_output_baton_t is simply wrong. At first glance it appears that what you really need to do is tweak the types of the other fields in context_saver_t, but at the moment I'm on the road and not in a position to prove that. In general, however, when fixing a warning it's often a good idea to consider the semantics of the surrounding code, not just make a minimal change to make the compiler shot up. -- Brane
