[syslinux] [PATCH 3/5] installers: MSVC compatibility fixes

Shao Miller sha0.miller at gmail.com
Sun Mar 6 17:21:11 PST 2016


On 3/6/2016 17:03, Pete Batard via Syslinux wrote:
> The problem was due to the following warning when compiling for 64-bit 
> using using the latest WDK (7600.16385.1), with warning level 3 (/W3):
>
> 1>c:\rufus\src\syslinux\libinstaller\syslxmod.c(44) : warning C4242: 
> '=' : conversion from 'sector_t' to 'unsigned int', possible loss of data

Then your patch seems like the most sensible solution, as this warning 
identifies that the two objects have different types.  That declaration 
context wasn't included in the patch; too far up. :)

> I used to compile Rufus with /W3 everywhere when using WDK, but I have 
> very recently reduced the warning level to /W2 for the Syslinux part 
> (see [1]) , so that I could align with your source without having to 
> also apply a bunch of extra patches due to WDK compilation breakage 
> without additional casts (e.g. [2], [3], bearing in mind that this is 
> a commit that removes those casts). I remember that you guys weren't 
> too receptive about adding those casts to the source, when I proposed 
> them 4 years ago [4]...

/W3 and other strict modes of translation in Microsoft's and others' C 
implementations are enjoyable, except when they're not.  I agree that 
it'd be nice to translate in every [worthy and] targeted implementation 
without incident.

Regarding [4]: The code expects that the shifted 'libfat_sector_t' value 
will certainly fit in the 31 value bits of an 'int32_t'.  The logic 
prior to that might even prove it, but we'd have to follow such things 
as whether or not 'fs->clustshift' is sanity-checked, etc.

If the warning is ugly and the cast is ugly, there are other solutions, 
including:

1. Increase the precision of 'cluster'.  It seems that there are bitwise 
operations performed, so this probably isn't a good idea.

2. Use a macro or inline function with a descriptive name to deal with 
the conversion.

3. Disable the Microsoft precision warning, since Syslinux does lots of 
this sort of thing.

That last option seems pretty simple, without moving entirely from /W3 
to /W2.  Could that work?  (C4242)

> With /W2, you are correct that WDK doesn't complain about this 
> specific line any more, so I can live with this part not being 
> applied. That is, unless you also want to consider people who may be 
> compiling part of the Syslinux source using WDK with /W3 (which I 
> wouldn't mind being able to reinstate for Rufus compilation, but in 
> that case, Syslinux would also need these additional cast patches).

Your separate-line-for-each patch seems like the most sensible solution.

--
Shao Miller
https://twitter.com/synthetel


More information about the Syslinux mailing list