Build sources with native Windows compilers.

[cfehse]

Scope

Make the source tree compile with native Windows C compilers like Embarcaderos clang based bcc32c compiler and Microsoft Visual Studio 2019. These compiler are fully c99 compliant and available for free. Therefore it may be an alternative for example to MinGW on Windows system.

The changes to the build system (main CMakeLists.txt file) are compatible to cmake 3.0.2 even though the compiler support for the Embarcadero compiler will need cmake 3.6.0 to work properly. To get the best support for Microsoft Visual Studio 2019 it seem recommended to use a recent cmake version like 3.19.* (recent as of the creation of this PR)

All changes are tested with a standard cmake 3.6.0 installation under linux using the default toolchain and on Windows using MinGW and the Embarcadero Free C++ Compiler 10.2 respectively Embarcadero C++ Builder Community Edition 10.3 (newer and commercial versions of RAD Studio/C++ Builder should work as well). Visual Studio 2019 ( version 16.8.4) was tested with cmake 3.19.3.

What has changed?

Source Files:

scantool_850.c

• Removed include <strings.h> - this is a POSIX include so the Windows compiles don't have it.
• The inclusion of strings.h was moved to diag.h (see below) where the Win32 specific handling for strcasecmp via the configuration define HAVE_STRCASECMP is already located.

diag_os_win.c

• The Embarcadero compiler (and older Borland compilers as well) do not define the _kbhit() function in conio.h. They have an inline definition for _getch() but traditionally the functions are only defined without the _ (the old Microsoft style). This leads to an linker error for the _kbhit symbol. To avoid this error I changed this source and the build system project (see below) to check for both definitions and use the non underscored version if the underscored version is not available. If under Windows none of the functions are detected the build system will raise a fatal error.

diag_l2.c

Change the initializer of the l2internal struct to remove a compiler waring under MSVC. The bool element init_done is now initialized with false instead of NULL.

diag.h

• On UNIX systems if HAVE_STRCASECMP is set the header strings.h is now included.
• The special handling for Microsoft compilers was changed. Microsoft's current C Compiler (_MSC_VER >= 1920) fully supports ANSI C c99. The snprintf function is c99 compatible (https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=msvc-160) since Visual Studio 2015. The CURFILE macro is supported as well. So all this special treatments now only apply to versions before Visual Studio 20219.
• The disabling for the MVSC compiler warning W4996 was move before the include of stdio.h to disable the warning effectively.

Build system:

CMakeLists.txt

• Added warning options for the Embarcadero Compiler - bcc32x (the clang based compiler using clang standard command line options) is here supported as well althought takes some tweaks to make cmake recognize this compiler correctly.
• Added the static C Runtime import library for the Embarcadero compilers. The default is to use the dynamic C Runtime dlls which must be present when running the binary.
• Added support for Visual Studio 2019 including linking against the static C runtime libary. The default is to use the dynamic C Runtime dlls which must be present when running the binary. The warning level for MSVC was set to level 2 (/W2) which should be equivalent to the current warning level of gcc. To add all type conversion warning during the build the warning level must be raised to level 3 (/W3) for MSVC and '-Wconversion' must be added for gcc (tested on MinGW).
• Added the symbol checks for the conio.h functions (see above).

cconf.h.in

• Populate the new defines HAVE_MS_KBHIT and HAVE_MS_GETCH.

build_simple.bat

Simple build scripts for convenience like the existing build_simple.sh. This script can serve as a starting point for new developers to get a fist impression about the build system.

.gitignore .dockerignore

• cmake 1.19 introduces so called presets defined in a JSON format. User presets can be stored in a file called CMakeUserPresets.json in the source directory. I find this a nice feature to manage multiple build configurations. This user preferences should not be version controlled so I added the file to the ignore files.
• I added the CMakeSettings.json file as well because Visual Studio used this file to manage there available configurations.
• Added IntelliJ IDEA, Visual Studio Code and Visual Studio directories to the ignore files.

Perhaps you like the changes and accept this merge request.

Thanks!

@fenugrec @nsajko

[fenugrec]

Hi,

thanks for your PR.

Not familiar with embarcadero... what advantages would it have over mingw for example ?

Some comments :

scantool_850.c

• it needs strings.h because that's where strcasecmp would be defined (not part of C99 AFAIK)
• I believe string.h is included indirectly via scantool.h (not diag.h), but I generally prefer that files include what they need without relying on indirect includes. freediag may be inconsistent in this regard, but that should be a separate PR if it bothers someone enough to make everything uniform.

The case of diag_os.h / diag.h is more debatable, but I don't want to get into that right now, and IMO it's out of scope for this PR. Unless those are causing problems with embarcadero support (in which case I'm happy to discuss this further), I'd ask that you keep the original structure.

kbhit etc in general I think what you've done should be ok, but I'd like links / references that confirm that the different _kbhit and _getch implementations we're trying to combine all behave the same, at least as far as diag_os_ipending() is concerned. This diag_os_ipending() should be testable in diag_test someday, without needing an active ECU connection...

build_*.bat Seems ok. I generally avoid building "in-tree", or even in a build/ subdirectory like build-simple.sh is doing; but on this simple project, I can find no technical reason to object.

[cfehse]

Hi,

thanks for the comments!

scantool_850.c

• It "needs" strings.h but e.g. on Linux strings.h is included by string.h (which on AIX, HP-UX or Solaris very likely would not be the case). So my code did compile on linux but generally speaking the unit should include strings.h.
• I moved the inclusion of strings.h to diag.h because there you process the configuration define HAVE_STRCASECMP anyway. I think that would be a good solution because all other units using strcasecmp do not directly include strings.h - in fact all these units get the definition currently via string.h on linux - but include diag.h to get the define for Windows.
• And I restored the include for diag_os.h.

kbhit et all

Borland does not use different implementations but different declarations. These are the same functions. I am even not sure if these functions were first introduced by Borland Turbo C or by Microsoft C for MS-DOS.

Borland Documentation: http://docwiki.embarcadero.com/RADStudio/Sydney/en/Kbhit http://docwiki.embarcadero.com/RADStudio/Sydney/en/Getch

Microsoft Documentation: https://docs.microsoft.com/de-de/cpp/c-runtime-library/reference/kbhit?view=msvc-160 https://docs.microsoft.com/de-de/cpp/c-runtime-library/reference/getch-getwch?view=msvc-160

I think the content of conio.h from MinGW illustrates the problem:

_CRTIMP int cdecl MINGW_NOTHROW _getch (void); _CRTIMP int cdecl MINGW_NOTHROW _getche (void); _CRTIMP int cdecl MINGW_NOTHROW _kbhit (void); _CRTIMP int cdecl MINGW_NOTHROW _putch (int); _CRTIMP int cdecl MINGW_NOTHROW _ungetch (int);

ifndef _NO_OLDNAMES

_CRTIMP int cdecl MINGW_NOTHROW getch (void); _CRTIMP int cdecl MINGW_NOTHROW getche (void); _CRTIMP int cdecl MINGW_NOTHROW kbhit (void); _CRTIMP int cdecl MINGW_NOTHROW putch (int); _CRTIMP int cdecl MINGW_NOTHROW ungetch (int);

endif / Not _NO_OLDNAMES /

Since 1998 Borland and Microsoft can not agree on what the old names are and what the new ones.

[cfehse]

Not familiar with embarcadero... what advantages would it have over mingw for example ?

Why Borland? Personal preference and I think especially the Embarcadero Free C++ Compiler 10.2 is currently the easiest way to get a good clang based native C/ C++ compiler on Windows. Extract the zip, set the PATH. Ready. Plattform SDK headers and import libraries are included as well. For Projects like this I find this ideal. These Windows Runtime function name confusion is annoying - and has been the last 20 years.

MinGW is good. But it is not a native Windows compiler. It is a UNIX compiler which can link against win32/win64 SDK APIs. That is a good thing. And it uses the MSYS layer to emulate some of the UNIX behavior on Windows (at least the versions I used in the past). That's all okay but I think to have more choices in the build system is not a disadvantage.

The code in this project is ANSI C c99 - so no fancy C++ features - and I did not needed any __BORLANDC__ defines but found quite some _MSC_VER ones (this is mostly due to the fact, that the bcc32c/x compilers are literally only frontends for LLVM).

[fenugrec]

• HAVE_STRCASECMP : fair point, I forgot I was doing some magic related to that in the .h

• kbhit : thanks. Making & testing win builds is getting more complicated for me (ironically, most users of this and libdiag are on win !) I'd probably add those two links somewhere as comments.

• borland : good to know. I've been trying to keep the code at a "minimum common denominator" of compilers, although MSVC has incomplete, broken C99 since forever.

But coming "soon" will be either some C11 atomics, pthreads garbage, or some other mechanism to fix the longstanding issues with the periodic timers (called asynchronously) breaking stuff and segfaulting. I assume, being clang-based, bcc32/64 should probably have proper C11 by now ? I'd very much like to avoid having a triple-implementation (c11, pthread, win API) of course.

In a perfect world I'd have some multi-platform CI tests compiling on every commit, but I don't have time to work on that.

[cfehse]

I assume, being clang-based, bcc32/64 should probably have proper C11 by now ?

No it looks not great in the regard. The C11 header are not included in the distributions I have - perhaps the latest RAD Studio version have them but that is to expensive I think.

I happened to have Visual Studio 2017 on my system (which I totally forgot about) and I installed LLVM 11.0.1 just for reference. I couldn't get neither of this compilers to compile the sources. The problem with the native clang is that it tries to emulate MSVC very closely and the macros in utlist.h don't like that at all. An other problem is that it requires to have a C Runtime and Platform SDK installed separately (which I had due to Visual Studio). All in all a lot of hassle to get 48 standard C99 sources compiled and linked on Windows.

[fenugrec]

No it looks not great in the regard. The C11 header are not included in the distributions I have

Hmm. So probably guaranteed to break again when I add C11.

just for reference. I couldn't get neither of this compilers to compile the sources. The problem with the native clang is that it tries to emulate MSVC very closely and the macros in utlist.h don't like that at all.

Ah interesting. I've had issues with MSVC for a while (see #54 , solved, and #56 ). My sentiment hasn't changed, that I don't want to bend over backwards to accomodate every C99 feature that MSVC refuses to support. But for utlist.h , we could check if it's been updated upstream as I believe they're testing it on MSVC and at least that should work.

But realistically,

• pretty much any version of MSVC is going to fail (I think the main block is the c99 designated initializers, anything else major ?)
• LLVM also fails (due to being halfway between a decent compiler and pretending to be MSVC, if I understand correctly ?)
• embarcadero should work, until C11 is required ?

[cfehse]

pretty much any version of MSVC is going to fail (I think the main block is the c99 designated initializers, anything else major ?)

So Visual Studio 2019 and up will do the trick: https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/

LLVM also fails (due to being halfway between a decent compiler and pretending to be MSVC, if I understand correctly ?)

LLVM (clang) on Windows will follow - because the Windows port is meant as a direct replacement for cl.exe (that's what they say on there website). LLVM will be ready to be a standard compiler on Windows as they are the standard compiler on Apple platforms.

embarcadero should work, until C11 is required ?

This is possible.

I don't know who the userbase of this software really is. My intension for this PR was to have a quick and effortless start for people compiling the package (perhaps the file build_mingw.bat is more valuable than all other changes. gg). If a Windows version for Windows developer is important the code must compile easily with Visual Studio I think.

If you don't have the time to test or don't want to integrate this PR - that's not a big deal for me. I learned something about cmake looked at some serious C sources and I can play on my fork und find an old car or some kind of simulator to play with.

The next PR I am trying to make is to include a proper manifest (and perhaps version info) into the Windows binaries. That's important on Windows and works with MinGW as well.

[cfehse]

msvc-2019-compile-freediag.txt

And just for grins and giggles - Visual Studio 2019 compiles the sources. See the attached text files. That was a 64 bit build with some warnings...

[fenugrec]

So Visual Studio 2019 and up will do the trick: https://devblogs.microsoft.com/cppblog/c11-and-c17-standard-support-arriving-in-msvc/

interesting. And also confusing... there should be a "supported compilers" section in the README - I can't keep track of all the subtleties...

I don't know who the userbase of this software really is.

Quite small, and getting smaller since freediag doesn't support any CAN-based ECUs which is becoming the norm on ~all recent models. Few people have compiled natively on windows over the years. I assume typical users download the mingw build (now horribly outdated), and others compile for linux. I really have no statistics for any of this, just going by feeling. The repo keeps getting forked / starred so there is definitely some interest...

The "backend" libdiag is also essential for another project I maintain (nisprog) that is a Nissan ECU reflasher. But in that case I'm also the one providing builds.

For me the effort of making this compileable on MSVC is too much, but I do merge reasonable PRs that don't change the whole codebase just to make MSVC happy. So I'm definitely interested in this PR if it adds an easy / easier option. But I will not test and troubleshoot embarcadero builds, I can only rely on bug reports and PRs.

something about cmake looked at some serious C sources

Hahaha "serious C sources", I'm flattered, but I can't say freediag is of exceptional code quality. I try my best , but it's a small project with few contributors...

The next PR I am trying to make is to include a proper manifest (and perhaps version info) into the Windows binaries. That's important on Windows and works with MinGW as well.

Cool. I'm almost ready to merge this PR, and if it breaks someday because of C11, we can cross that bridge when we get there.

[cfehse]

Hi,

I looked a bit closer into diag.h for Visual Studio 2019 (snprintf is c99 compatible in VS2019 CURFILE will work and the "secure warning" can be switched off, if set before including stdio.h) and this is what's left for warning for a build:

`

ninja [13/47] Building C object scantool\CMakeFiles\diag.dir\diag_l2_iso14230.c.obj ..\scantool\diag_l2_iso14230.c(781): warning C4018: ">": Konflikt zwischen "signed" und "unsigned" [16/47] Building C object scantool\CMakeFiles\diag.dir\diag_tty_win.c.obj ..\scantool\diag_tty_win.c(523): warning C4244: "=": Konvertierung von "LONGLONG" in "long", möglicher Datenverlust ..\scantool\diag_tty_win.c(524): warning C4244: "=": Konvertierung von "LONGLONG" in "long", möglicher Datenverlust ..\scantool\diag_tty_win.c(528): warning C4244: "=": Konvertierung von "LONGLONG" in "long", möglicher Datenverlust [18/47] Building C object scantool\CMakeFiles\diag.dir\diag_os_win.c.obj ..\scantool\diag_os_win.c(130): warning C4244: "=": Konvertierung von "double" in "float", möglicher Datenverlust [19/47] Building C object scantool\CMakeFiles\diag.dir\diag_l2_vag.c.obj ..\scantool\diag_l2_vag.c(217): warning C4244: "=": Konvertierung von "unsigned int64" in "int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(228): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "unsigned int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(302): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(589): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "unsigned int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(645): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "unsigned int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(666): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "unsigned int", möglicher Datenverlust ..\scantool\diag_l2_vag.c(674): warning C4244: "Funktion": Konvertierung von "unsigned int64" in "unsigned int", möglicher Datenverlust [20/47] Building C object scantool\CMakeFiles\diag.dir\diag_l2_saej1850.c.obj ..\scantool\diag_l2_saej1850.c(266): warning C4244: "=": Konvertierung von "unsigned int64" in "unsigned long", möglicher Datenverlust [21/47] Building C object scantool\CMakeFiles\diag.dir\diag_l2.c.obj ..\scantool\diag_l2.c(57): warning C4047: "Initialisierung": Anzahl der Dereferenzierungen bei "bool" und "void *" unterschiedlich [31/47] Building C object scantool\CMakeFiles\diag.dir\diag_l7_kwp71.c.obj ..\scantool\diag_l7_kwp71.c(143): warning C4244: "=": Konvertierung von "uint16_t" in "uint8_t", möglicher Datenverlust [40/47] Building C object scantool\CMakeFiles\freediag.dir\scantool_850.c.obj ..\scantool\scantool_850.c(355): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(767): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(775): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(803): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(819): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(835): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(859): warning C4244: "=": Konvertierung von "unsigned long" in "uint16_t", möglicher Datenverlust ..\scantool\scantool_850.c(967): warning C4244: "Funktion": Konvertierung von "uint16_t" in "uint8_t", möglicher Datenverlust [47/47] Linking C executable scantool\freediag.exe `

From a first look there is some type confusion in the code I think. (Oh my compiler speeds german is see - I hope you get the information)

[cfehse]

Hi,

I added support for Visual Studio 2019 to the cmake system and diag.h. I haven't rebased the commits yet for a better view of the additional changes made. The support for VS was quite simple because the compiler has a really good support for c99.

I will update the PR description later to reflect the new changes and the extended scope. If you have the time just take a look.

Thanks!

@fenugrec

[cfehse]

Hi,

I squashed the commits together because the PR is pretty ready now. I removed the build scripts and added only the build_simple.bat (used MinGW) . The other scripts are not needed because Visual Studio and Visual Studio Code have very good support for cmake simply by opening the freediag base directory in the IDE. All the cmake magic can than be done from the IDE itself.

@fenugrec

[fenugrec]

I'll cherry-pick part of this commit (those related to strcasecmp) as they arent really related to windows. Meanwhile, can you check your line-endings (should be LF-only, although I can probably force it at my end), and trailing whitespace, there seems to be a few minor inconsistencies

[fenugrec]

Merged in 618c4681444330a06ff763b56018ce1a82fedaef , please test to make sure I didn't break anything in the split !

Thanks.

[cfehse]

Merged in 618c468 , please test to make sure I didn't break anything in the split !

For me the new master branch configures/compiles fine with:

• Visual Studio 2019 (x86/x64) (integrated cmake and cmake 3.19.3)
• Visual Studio Code on Windows (MinGW, MinGW64, BCC, MSVC) (cmake 3.6.0 and higher)
• Visual Studio Code remote on WSL (cmake 3.13.4 default installation debian)
• Debian (cmake 3.13.4 default installation debian)

Looks good to me.

Thanks!