Updates to generate windows library #103
Conversation
|
DDvO
left a comment
DDvO
left a comment
There was a problem hiding this comment.
Thank you for your contribution.
Here are a couple of questions and suggestions.
Can you please also add a Windows CI build to .github/workflows/ ?
| static const char *const GENCMP_NAME = "genCMPClient"; | ||
| static const size_t loc_len = 256; | ||
| #define loc_len 256 | ||
|
|
||
| /*!< these variables are shared between threads */ | ||
| static LOG_cb_t LOG_fn = 0; | ||
| static const char *app_name = GENCMP_NAME; | ||
| static severity verbosity = LOG_WARNING; | ||
| static const char *app_name = "genCMPClient"; | ||
| static severity verbosity = 4; /* LOG_WARNING equivalent */ |
There was a problem hiding this comment.
Are these changes because the M$ C compiler cannot make full use of constants, where #define can be used as a workaround?
If so, please adapt:
| static const char *const GENCMP_NAME = "genCMPClient"; | |
| static const size_t loc_len = 256; | |
| #define loc_len 256 | |
| /*!< these variables are shared between threads */ | |
| static LOG_cb_t LOG_fn = 0; | |
| static const char *app_name = GENCMP_NAME; | |
| static severity verbosity = LOG_WARNING; | |
| static const char *app_name = "genCMPClient"; | |
| static severity verbosity = 4; /* LOG_WARNING equivalent */ | |
| #define GENCMP_NAME "genCMPClient" | |
| #define LOC_LEN 256 | |
| /*!< these variables are shared between threads */ | |
| static LOG_cb_t LOG_fn = 0; | |
| static const char *app_name = GENCMP_NAME; | |
| static severity verbosity = LOG_WARNING; |
| if(MSVC) | ||
| # MSVC compiler flags | ||
| add_compile_options(/W4) # High warning level | ||
| add_compile_options(/wd4996) # Disable deprecated function warnings |
There was a problem hiding this comment.
I'd be interested to know what happens without this option - which function(s) would be flagged as deprecated?
| if(NOT DEFINED GENCMP_NO_SECUTILS) | ||
| add_compile_options(-Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-shadow) | ||
| endif() |
There was a problem hiding this comment.
I presume that you build using GENCMP_NO_SECUTILS because libSecUtils does not build for Windows, right?
If so, I suggest adding to the the end of the if(MSVC) branch, e.g.:
if(NOT DEFINED GENCMP_NO_SECUTILS)
message(FATAL_ERROR "Windows build is not (yet) supported for libSecUtils")
endif()
|
Ah, just realized that @rajeev-0 had already been working on this and earlier today rebased his branch on your work: master...add_windows_CI |
Motivation
(Please write out your motivation here.)
Proposed Changes
BEFORE (causing error)
set(USERS "^(\w:)?\Users\")
AFTER (fixed)
set(USERS "^([a-zA-Z]:)?\\Users\\")
GCC-specific compiler flags like -Woverflow, -pedantic not supported by MSVC. Added following.
if(MSVC)
MSVC compiler flags
add_compile_options(/W4) # High warning level
add_compile_options(/wd4996) # Disable deprecated function warnings
add_compile_definitions(_CRT_SECURE_NO_WARNINGS)
else()
GCC/Clang compiler flags
add_compile_options(-pedantic)
add_compile_options(
-Wall -Woverflow -Wextra -Wmissing-prototypes -Wstrict-prototypes -Wswitch
-Wsign-compare -Wformat -Wtype-limits -Wundef -Wconversion -Wunused-parameter)
add_compile_options(-Wno-c99-extensions -Wno-language-extension-token -Wno-declaration-after-statement -Wno-expansion-to-defined)
if(NOT DEFINED GENCMP_NO_SECUTILS)
add_compile_options(-Wno-sign-conversion -Wno-shorten-64-to-32 -Wno-shadow)
endif()
endif()
2. • Added stub for syslog for windows: src/genericCMPClient_util.c - #include <syslog.h> not available on Windows
3. Used string literals for const- Problem in using macros as a constant string. For example, following does not work on windows.
static const char *app_name = GENCMP_NAME;
static severity verbosity = LOG_WARNING;
Test Plan
(Please provide clear instructions on how to verify that your changes work.)