Pete Batard
2016-Mar-06 15:47 UTC
[syslinux] [PATCH 3/5] installers: MSVC compatibility fixes
On 2016.03.06 13:13, Gene Cumm wrote:> Did Visual Studio actually complain about this one?WDK compiler (which I also use) if I recall correctly. At any rate, some older compilers do not like double initializations like this one, and I don't think this change should be much of a contention point, since it doesn't introduce any liability. Regards, /Pete
Shao Miller
2016-Mar-06 20:34 UTC
[syslinux] [PATCH 3/5] installers: MSVC compatibility fixes
On 3/6/2016 10:47, Pete Batard via Syslinux wrote:> On 3/6/2016 08:13, Gene Cumm via Syslinux wrote: >>> - len = lba = 0; >>> >+ len = 0; >>> >+ lba = 0; >>> > >>> > memset_sl(ex, 0, nptrs * sizeof *ex); >> Did Visual Studio actually complain about this one? > > WDK compiler (which I also use) if I recall correctly. At any rate, > some older compilers do not like double initializations like this one, > and I don't think this change should be much of a contention point, > since it doesn't introduce any liability.If those compilers are newer than 1989, they would certainly be broken for complaining about such an initialization, as it's well-defined. Windows DDK 6001.18001 does not complain. Visual Studio 6 doesn't, either. If a known implementation that Syslinux is targeting complains, could you please share which one it is? This quirk is so surprising that I find it likely for that implementation to have other quirks that we might have to be on the look-out for. I'd like to research them, as C portability is a strong interest of mine. If this change is simply due to a mental note about an incident where a compiler once complained about this type of thing, I think that times have changed and the note no longer applies. :) I appreciate your contributions quite a bit, Pete! - Shao Miller of Synthetel
Pete Batard
2016-Mar-06 22:03 UTC
[syslinux] [PATCH 3/5] installers: MSVC compatibility fixes
Hi Shao, You're right, "a=b=<immediate value>;" wasn't the actual issue. On 2016.03.06 20:34, Shao Miller via Syslinux wrote:> If this change is simply due to a mental note about an incident where a > compiler once complained about this type of thingThe 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 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]... 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). Regards, /Pete [1] https://github.com/pbatard/rufus/commit/db0880e534e4e9504e8c283256fbfbd0683abded#diff-729493b04f2117d4290873c20e986726 [2] https://github.com/pbatard/rufus/commit/db0880e534e4e9504e8c283256fbfbd0683abded#diff-9b695506cac9f006f20d03d23ee289e0 [3] https://github.com/pbatard/rufus/commit/db0880e534e4e9504e8c283256fbfbd0683abded#diff-55f6a1a381f24361820e0047aec04222 [4] http://www.syslinux.org/archives/2012-February/017643.html