dali/bacnet_stack
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Make clean build with MSVC /Wall (#740)

Browse Source 
* ci: Fix compile warning as errors was not correct

We want to enable warning as errors both Windows and Linux. This is
easiest to do with cmake option as -Werror does not work with MSVC. Also
it is self explaining what it does so no comment needed.

* dlmstp_linux: Fix -Wdeclaration-after-statement compiler warnings

Make dlmstp_linux C89/C90 combatible

```
/bacnet-stack/ports/linux/dlmstp_linux.c: In function ‘Timer_Silence’:
/bacnet-stack/ports/linux/dlmstp_linux.c:56:5: warning:
 ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
   56 |     int32_t res;
      |     ^~~~~~~
/bacnet-stack/ports/linux/dlmstp_linux.c: In function ‘dlmstp_init’:
/bacnet-stack/ports/linux/dlmstp_linux.c:795:5: warning:
 ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
  795 |     struct termios newtio;
      |     ^~~~~~
```

* Fix warnings produces by MSVC

Now that we have enabled /Wall for MSVC we get some warnings with it
which can be easily fixed.

We get following warnings:

```
src\bacnet\bacstr.c(223,39): warning C4127: conditional expression is constant
apps\router-mstp\main.c(1123,1): warning C4702: unreachable code
apps\epics\main.c(885,53): warning C4459: declaration of 'myState' hides global declaration
```

* cmake: Use /Wall with MSVC

Make MSVC to build cleanly with Wall. This might matter for some Windows
developers. And you never know if MSVC will find more bugs.

* cmake: Improve router build

Router build gives some warnings as it is not C90 compatible. It is ok
that example is not following C90 rules. Also it is annoing to new user
to build this cmake as first error usually is that libconfig is not
found. Let's just give warning about this so first build will usually go
smoother.

---------

Co-authored-by: Kari Argillander <kari.argillander@fidelix.com>
This commit is contained in:
Kari Argillander
2024-08-22 15:50:20 +03:00
committed by GitHub
parent 76d605e06b
commit 9662a76ebd
9 changed files with 106 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/main.yml
+2 -6
View File
@@ -56,18 +56,14 @@ jobs:
source_dir=$GITHUB_WORKSPACE
fi
c_flags=""
if [[ "$RUNNER_OS" != "Windows" ]]; then
# Warnings as errors.
c_flags="$c_flags -Werror"
fi
cmake_options="-DCMAKE_COMPILE_WARNING_AS_ERROR=ON"
# Compile as much as possible to at least compile test code.
c_flags="$c_flags -DBACFILE=ON"
c_flags="$c_flags -DPRINT_ENABLED=1"
c_flags="$c_flags -DBACNET_TIME_MASTER=ON"
c_flags="$c_flags -DBACAPP_COLOR_RGB_CONVERSION_ENABLED=ON"
cmake_options="-DCMAKE_C_FLAGS=$c_flags"
cmake_options="$cmake_options -DCMAKE_C_FLAGS=$c_flags"
# TODO: Add BACDL_BIP6=ON when it builds withous errors.
cmake_options="$cmake_options -DBACDL_BIP=ON"
CMakeLists.txt
+71 -4
View File
@@ -138,6 +138,36 @@ if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "AppleCla
add_compile_options(-Wno-float-conversion)
add_compile_options(-Wno-missing-declarations)
add_compile_options(-Wno-unused-but-set-variable)
elseif(MSVC)
add_compile_options(/Wall)
# Potentially uninitialized local variable. Definetly fix these. There might
# be false positives but it is better to fix them also.
add_compile_options(/wd4701)
# Function is unsafe (strtok, sscanf, fopen, strerror).
add_compile_options(/wd4996)
# Handle all enums values in switch.
add_compile_options(/wd4061)
# Possible loss of data.
add_compile_options(/wd4242 /wd4244 /wd4267)
# Signed/unsigned mismatch.
add_compile_options(/wd4018 /wd4388 /wd4389)
# Not defined as a preprocessor macro.
add_compile_options(/wd4668)
# Function not inlined.
add_compile_options(/wd4710)
# Compiler adds padding to structures.
add_compile_options(/wd4820)
# Might be slower if builded with /Qspectre
add_compile_options(/wd5045)
endif()
if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "AppleClang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU")
@@ -618,10 +648,6 @@ elseif(WIN32)
$<$<BOOL:${BACDL_BIP}>:ws2_32>
$<$<BOOL:${BACDL_BIP}>:iphlpapi>)
if(MSVC)
add_definitions("/W3 /D_CRT_SECURE_NO_WARNINGS /wd4244 /wd4018 /wd4267")
endif()
target_sources(${PROJECT_NAME} PRIVATE
ports/win32/bacport.h
$<$<BOOL:${BACDL_BIP6}>:ports/win32/bip6.c>
@@ -712,9 +738,24 @@ if(BACNET_STACK_BUILD_APPS)
if(BACDL_MSTP)
add_executable(mstpcap apps/mstpcap/main.c)
target_link_libraries(mstpcap PRIVATE ${PROJECT_NAME})
target_compile_options(mstpcap PRIVATE
# NOTE: Might be that this example currently doesn't work on Windows because
# of the following warning:
# 'strncasecmp': macro redefinition
$<$<C_COMPILER_ID:MSVC>:/wd4005>
# 'gettimeofday' undefined; assuming extern returning int
$<$<C_COMPILER_ID:MSVC>:/wd4013>
)
add_executable(mstpcrc apps/mstpcrc/main.c)
target_link_libraries(mstpcrc PRIVATE ${PROJECT_NAME})
target_compile_options(mstpcrc PRIVATE
# NOTE: Might be that this example currently doesn't work on Windows because
# of the following warning:
# 'gettimeofday' undefined; assuming extern returning int
$<$<C_COMPILER_ID:MSVC>:/wd4013>
)
endif()
if(BACNET_BUILD_PIFACE_APP)
@@ -729,6 +770,10 @@ if(BACNET_STACK_BUILD_APPS)
src/bacnet/basic/client/bac-data.c
src/bacnet/basic/client/bac-rw.c)
target_link_libraries(bacpoll PRIVATE ${PROJECT_NAME})
target_compile_options(bacpoll PRIVATE
# Unreachable code because we have endless loop.
$<$<C_COMPILER_ID:MSVC>:/wd4702>
)
endif(BACNET_BUILD_BACPOLL_APP)
if(BACNET_BUILD_BACDISCOVER_APP)
@@ -737,6 +782,10 @@ if(BACNET_STACK_BUILD_APPS)
src/bacnet/basic/client/bac-discover.c
src/bacnet/basic/client/bac-rw.c)
target_link_libraries(bacdiscover PRIVATE ${PROJECT_NAME})
target_compile_options(bacdiscover PRIVATE
# Unreachable code because we have endless loop.
$<$<C_COMPILER_ID:MSVC>:/wd4702>
)
endif(BACNET_BUILD_BACDISCOVER_APP)
if(BACDL_BIP OR BACDL_BIP6)
@@ -766,6 +815,11 @@ if(BACNET_STACK_BUILD_APPS)
target_link_libraries(reinit PRIVATE ${PROJECT_NAME})
if(BACDL_MSTP AND NOT WIN32)
find_library(LIBCONFIG_LIBRARIES NAMES config)
if(NOT LIBCONFIG_LIBRARIES)
message(WARNING "BACNET: Will not build apps/router as libconfig not found")
return()
else()
add_executable(
router
apps/router/ipmodule.c
@@ -785,6 +839,15 @@ if(BACNET_STACK_BUILD_APPS)
PRIVATE ${PROJECT_NAME}
# needs libconfig
-lconfig)
target_compile_options(router PRIVATE
# These make this example not totally C90 compatible but it is ok.
-Wno-declaration-after-statement
-Wno-overlength-strings
-Wno-variadic-macros
)
endif()
endif()
if(BACDL_BIP6)
@@ -797,6 +860,10 @@ if(BACNET_STACK_BUILD_APPS)
add_executable(server apps/server/main.c)
target_link_libraries(server PRIVATE ${PROJECT_NAME})
target_compile_options(server PRIVATE
# Unreachable code because we have endless loop.
$<$<C_COMPILER_ID:MSVC>:/wd4702>
)
add_executable(timesync apps/timesync/main.c)
target_link_libraries(timesync PRIVATE ${PROJECT_NAME})
apps/epics/main.c
+7 -7
View File
@@ -876,13 +876,13 @@ static uint8_t Read_Properties(
* If the present state is GET_HEADING_RESPONSE, store the results
* in globals for later use.
* @param rpm_data [in] The list of RPM data received.
* @param myState [in] The current state.
* @param state [in] The current state.
* @return The next state of the EPICS state machine, normally NEXT_OBJECT
* if the RPM got good data, or GET_PROPERTY_REQUEST if we have to
* singly process the list of Properties.
*/
static EPICS_STATES ProcessRPMData(
BACNET_READ_ACCESS_DATA *rpm_data, EPICS_STATES myState)
BACNET_READ_ACCESS_DATA *rpm_data, EPICS_STATES state)
{
BACNET_READ_ACCESS_DATA *old_rpm_data;
BACNET_PROPERTY_REFERENCE *rpm_property;
@@ -890,7 +890,7 @@ static EPICS_STATES ProcessRPMData(
BACNET_APPLICATION_DATA_VALUE *value;
BACNET_APPLICATION_DATA_VALUE *old_value;
bool bSuccess = true;
EPICS_STATES nextState = myState; /* assume no change */
EPICS_STATES nextState = state; /* assume no change */
/* Some flags to keep the output "pretty" -
* wait and put these object lists at the end */
bool bHasObjectList = false;
@@ -902,7 +902,7 @@ static EPICS_STATES ProcessRPMData(
while (rpm_property) {
/* For the GET_LIST_OF_ALL_RESPONSE case,
* just keep what property this was */
if (myState == GET_LIST_OF_ALL_RESPONSE) {
if (state == GET_LIST_OF_ALL_RESPONSE) {
switch (rpm_property->propertyIdentifier) {
case PROP_OBJECT_LIST:
bHasObjectList = true; /* Will append below */
@@ -924,7 +924,7 @@ static EPICS_STATES ProcessRPMData(
value = value->next;
free(old_value);
}
} else if (myState == GET_HEADING_RESPONSE) {
} else if (state == GET_HEADING_RESPONSE) {
Property_Value_List[i++].value = rpm_property->value;
/* copy this pointer.
* On error, the pointer will be null
@@ -946,10 +946,10 @@ static EPICS_STATES ProcessRPMData(
}
/* Now determine the next state */
if (myState == GET_HEADING_RESPONSE) {
if (state == GET_HEADING_RESPONSE) {
nextState = PRINT_HEADING;
/* press ahead with or without the data */
} else if (bSuccess && (myState == GET_ALL_RESPONSE)) {
} else if (bSuccess && (state == GET_ALL_RESPONSE)) {
nextState = NEXT_OBJECT;
} else if (bSuccess) { /* and GET_LIST_OF_ALL_RESPONSE */
/* Now append the properties we waited on. */
apps/mstpcap/main.c
-2
View File
@@ -895,8 +895,6 @@ static BOOL WINAPI CtrlCHandler(DWORD dwCtrlType)
Sleep(100);
}
exit(0);
return TRUE;
}
#else
static void sig_int(int signo)
apps/router-mstp/main.c
-2
View File
@@ -1119,8 +1119,6 @@ static BOOL WINAPI CtrlCHandler(DWORD dwCtrlType)
Sleep(100);
}
exit(0);
return TRUE;
}
static void control_c_hooks(void)
ports/linux/dlmstp_linux.c
+2 -3
View File
@@ -42,6 +42,7 @@
}
static uint32_t Timer_Silence(void *poPort)
{
int32_t res;
struct timeval now, tmp_diff;
SHARED_MSTP_DATA *poSharedData;
struct mstp_port_struct_t *mstp_port = (struct mstp_port_struct_t *)poPort;
@@ -53,8 +54,6 @@ static uint32_t Timer_Silence(void *poPort)
return -1;
}
int32_t res;
gettimeofday(&now, NULL);
timersub(&poSharedData->start, &now, &tmp_diff);
res = ((tmp_diff.tv_sec) * 1000 + (tmp_diff.tv_usec) / 1000);
@@ -764,6 +763,7 @@ bool dlmstp_init(void *poPort, char *ifname)
unsigned long hThread = 0;
int rv = 0;
SHARED_MSTP_DATA *poSharedData;
struct termios newtio;
struct mstp_port_struct_t *mstp_port = (struct mstp_port_struct_t *)poPort;
if (!mstp_port) {
return false;
@@ -792,7 +792,6 @@ bool dlmstp_init(void *poPort, char *ifname)
exit(1);
}
struct termios newtio;
printf("RS485: Initializing %s", poSharedData->RS485_Port_Name);
/*
Open device for reading and writing.
ports/win32/bacport.h
+3 -3
View File
@@ -38,10 +38,10 @@
#ifdef __MINGW32__
#include <ws2spi.h>
#else
#pragma warning( push )
#pragma warning(disable: 4101 4191)
#include <wspiapi.h>
/* Microsoft has deprecated some CRT and C++ Standard Library functions
and globals in favor of more secure versions. */
#pragma warning(disable : 4996)
#pragma warning( pop )
/* add winmm.lib to our build */
#pragma comment(lib, "winmm.lib")
#endif
src/bacnet/bacstr.c
+1 -5
View File
@@ -220,11 +220,7 @@ bool bitstring_set_bits_used(
unsigned bitstring_bits_capacity(BACNET_BIT_STRING *bit_string)
{
if (bit_string) {
if ((MAX_BITSTRING_BYTES * 8) <= (UINT8_MAX + 1)) {
return (MAX_BITSTRING_BYTES * 8);
} else {
return (UINT8_MAX + 1);
}
return min((MAX_BITSTRING_BYTES * 8), (UINT8_MAX + 1));
} else {
return 0;
}
src/bacnet/basic/sys/platform.h
-6
View File
@@ -106,10 +106,4 @@ __inline int c99_snprintf(char *outBuf, size_t size, const char *format, ...)
#define BACNET_STACK_FALLTHROUGH() /* fall through */
#endif
#if defined(_MSC_VER)
/* Silence the warnings about unsafe versions of library functions */
/* as we need to keep the code portable */
#pragma warning(disable : 4996)
#endif
#endif