From 40c5570d64fbbf164aaef3194a74a9b171de2c42 Mon Sep 17 00:00:00 2001 From: Kari Argillander Date: Thu, 15 Aug 2024 15:23:04 +0300 Subject: [PATCH] Force C89/C90 compatible and for test C99 (#722) * bacint: Do not use ULL suffix For sake of be more compatible with C89/C90 let's not use ULL at all. Overall conversion functions are lot cleaner now. Only idiotic thing is in bacnet_unsigned_length() where we need to do shift. There is probably better way to do that but could not come up any at resonable time. * Force C89/C90 and for tests C99 bacnet-stack seems to be all compatible with C89/C90. This is probably design choice. Let's force this in CMake so no one will break that by accident. In tests we are using some C99 features already. Let's not be to strict about those as those are "just tests". * Fix -Wdeclaration-after-statement warnings To make code C89/C90 compatible fix -Wdeclaration-after-statement warnings. We got like following warning without this change. ```C bacnet-stack/apps/epics/main.c:293:5: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement 293 | uint32_t Object_Instance; | ^~~~~~~~ ``` * cmake: Add -Wno-c99-extensions compiler option Clang does not like _Bool which is used in stdbool.h files. For now let's just ignore this. We could define that differently but let's think that another time. Now goal is to get warning free CI with more code to be builded in there. * cmake: Add -Wno-long-long compiler option for apple Apple seems to do stupid things with their system header. There is UINT64_MAX with ULL suffix and not like in Linux and Windows __UINT64_C(18446744073709551615) For this reason we need to ignore Wlong-long for it. --------- Co-authored-by: Kari Argillander --- CMakeLists.txt | 13 ++- apps/epics/main.c | 7 +- ports/bsd/bip-init.c | 3 +- src/bacnet/bacint.c | 211 ++++++++++++++++++++----------------------- test/CMakeLists.txt | 3 + 5 files changed, 117 insertions(+), 120 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 95c5d34b..2160fdad 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -101,6 +101,8 @@ if(BACNET_STACK_DEPRECATED_DISABLE) add_definitions(-DBACNET_STACK_DEPRECATED_DISABLE) endif() +set(CMAKE_C_STANDARD 90) + if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "AppleClang" OR CMAKE_C_COMPILER_ID MATCHES "GNU") add_compile_options(-Wall -Wextra -pedantic) # Add more warnings @@ -116,9 +118,18 @@ if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "AppleCla add_compile_options(-Wno-sign-conversion -Wno-conversion) add_compile_options(-Wno-sign-compare) - # Just noise from clang if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "AppleClang") + # Just noise from clang add_compile_options(-Wno-gnu-zero-variadic-macro-arguments) + + # Clang does not like _Bool which is not C90. We could fix this at some + # point. + add_compile_options(-Wno-c99-extensions) + endif() + + if (APPLE) + # Apple defines UINT64_MAX with ULL suffix. This is not C90 compliant. + add_compile_options(-Wno-long-long) endif() # Should be fixed in a future. diff --git a/apps/epics/main.c b/apps/epics/main.c index c5861da2..b198e0e0 100644 --- a/apps/epics/main.c +++ b/apps/epics/main.c @@ -287,11 +287,14 @@ static void MyReadPropertyMultipleAckHandler(uint8_t *service_request, static void Init_Service_Handlers(void) { - Device_Init(NULL); - #if BAC_ROUTING uint32_t Object_Instance; BACNET_CHARACTER_STRING name_string; +#endif + + Device_Init(NULL); + +#if BAC_ROUTING /* Put this client Device into the Routing table (first entry) */ Object_Instance = Device_Object_Instance_Number(); Device_Object_Name(Object_Instance, &name_string); diff --git a/ports/bsd/bip-init.c b/ports/bsd/bip-init.c index dcbd3f12..81f452a3 100644 --- a/ports/bsd/bip-init.c +++ b/ports/bsd/bip-init.c @@ -518,8 +518,7 @@ int bip_get_local_netmask(struct in_addr *netmask) if (ifname == NULL) ifname = "en0"; printf("ifname %s", ifname); - char *request = "netmask"; - rv = get_local_address(ifname, netmask, request); + rv = get_local_address(ifname, netmask, "netmask"); return rv; } diff --git a/src/bacnet/bacint.c b/src/bacnet/bacint.c index c185b725..5393f1ae 100644 --- a/src/bacnet/bacint.c +++ b/src/bacnet/bacint.c @@ -16,8 +16,8 @@ int encode_unsigned16(uint8_t *apdu, uint16_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff00) >> 8); - apdu[1] = (uint8_t)(value & 0x00ff); + apdu[0] = (uint8_t)((value >> 8) & 0xFF); + apdu[1] = (uint8_t)((value >> 0) & 0xFF); } return 2; @@ -26,8 +26,8 @@ int encode_unsigned16(uint8_t *apdu, uint16_t value) int decode_unsigned16(uint8_t *apdu, uint16_t *value) { if (apdu && value) { - *value = (uint16_t)((((uint16_t)apdu[0]) << 8) & 0xff00); - *value |= ((uint16_t)(((uint16_t)apdu[1]) & 0x00ff)); + *value = (uint16_t)apdu[0] << 8; + *value |= (uint16_t)apdu[1]; } return 2; @@ -36,9 +36,9 @@ int decode_unsigned16(uint8_t *apdu, uint16_t *value) int encode_unsigned24(uint8_t *apdu, uint32_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff0000) >> 16); - apdu[1] = (uint8_t)((value & 0x00ff00) >> 8); - apdu[2] = (uint8_t)(value & 0x0000ff); + apdu[0] = (uint8_t)((value >> 16) & 0xFF); + apdu[1] = (uint8_t)((value >> 8) & 0xFF); + apdu[2] = (uint8_t)((value >> 0) & 0xFF); } return 3; @@ -47,9 +47,9 @@ int encode_unsigned24(uint8_t *apdu, uint32_t value) int decode_unsigned24(uint8_t *apdu, uint32_t *value) { if (apdu && value) { - *value = ((uint32_t)((((uint32_t)apdu[0]) << 16) & 0x00ff0000)); - *value |= (uint32_t)((((uint32_t)apdu[1]) << 8) & 0x0000ff00); - *value |= ((uint32_t)(((uint32_t)apdu[2]) & 0x000000ff)); + *value = (uint32_t)apdu[0] << 16; + *value |= (uint32_t)apdu[1] << 8; + *value |= (uint32_t)apdu[2]; } return 3; @@ -58,10 +58,10 @@ int decode_unsigned24(uint8_t *apdu, uint32_t *value) int encode_unsigned32(uint8_t *apdu, uint32_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff000000) >> 24); - apdu[1] = (uint8_t)((value & 0x00ff0000) >> 16); - apdu[2] = (uint8_t)((value & 0x0000ff00) >> 8); - apdu[3] = (uint8_t)(value & 0x000000ff); + apdu[0] = (uint8_t)((value >> 24) & 0xFF); + apdu[1] = (uint8_t)((value >> 16) & 0xFF); + apdu[2] = (uint8_t)((value >> 8) & 0xFF); + apdu[3] = (uint8_t)((value >> 0) & 0xFF); } return 4; @@ -70,10 +70,10 @@ int encode_unsigned32(uint8_t *apdu, uint32_t value) int decode_unsigned32(uint8_t *apdu, uint32_t *value) { if (apdu && value) { - *value = ((uint32_t)((((uint32_t)apdu[0]) << 24) & 0xff000000)); - *value |= ((uint32_t)((((uint32_t)apdu[1]) << 16) & 0x00ff0000)); - *value |= ((uint32_t)((((uint32_t)apdu[2]) << 8) & 0x0000ff00)); - *value |= ((uint32_t)(((uint32_t)apdu[3]) & 0x000000ff)); + *value = (uint32_t)apdu[0] << 24; + *value |= (uint32_t)apdu[1] << 16; + *value |= (uint32_t)apdu[2] << 8; + *value |= (uint32_t)apdu[3]; } return 4; @@ -89,11 +89,11 @@ int decode_unsigned32(uint8_t *apdu, uint32_t *value) int encode_unsigned40(uint8_t *buffer, uint64_t value) { if (buffer) { - buffer[0] = (uint8_t)((value & 0x000000ff00000000ULL) >> 32); - buffer[1] = (uint8_t)((value & 0x00000000ff000000ULL) >> 24); - buffer[2] = (uint8_t)((value & 0x0000000000ff0000ULL) >> 16); - buffer[3] = (uint8_t)((value & 0x000000000000ff00ULL) >> 8); - buffer[4] = (uint8_t)(value & 0x00000000000000ffULL); + buffer[0] = (uint8_t)((value >> 32) & 0xFF); + buffer[1] = (uint8_t)((value >> 24) & 0xFF); + buffer[2] = (uint8_t)((value >> 16) & 0xFF); + buffer[3] = (uint8_t)((value >> 8) & 0xFF); + buffer[4] = (uint8_t)((value >> 0) & 0xFF); } return 5; @@ -108,15 +108,11 @@ int encode_unsigned40(uint8_t *buffer, uint64_t value) int decode_unsigned40(uint8_t *buffer, uint64_t *value) { if (buffer && value) { - *value = - ((uint64_t)((((uint64_t)buffer[0]) << 32) & 0x000000ff00000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[1]) << 24) & 0x00000000ff000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[2]) << 16) & 0x0000000000ff0000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[3]) << 8) & 0x000000000000ff00ULL)); - *value |= ((uint64_t)(((uint64_t)buffer[4]) & 0x00000000000000ffULL)); + *value = (uint64_t)buffer[0] << 32; + *value |= (uint64_t)buffer[1] << 24; + *value |= (uint64_t)buffer[2] << 16; + *value |= (uint64_t)buffer[3] << 8; + *value |= (uint64_t)buffer[4]; } return 5; @@ -131,12 +127,12 @@ int decode_unsigned40(uint8_t *buffer, uint64_t *value) int encode_unsigned48(uint8_t *buffer, uint64_t value) { if (buffer) { - buffer[0] = (uint8_t)((value & 0x0000ff0000000000ULL) >> 40); - buffer[1] = (uint8_t)((value & 0x000000ff00000000ULL) >> 32); - buffer[2] = (uint8_t)((value & 0x00000000ff000000ULL) >> 24); - buffer[3] = (uint8_t)((value & 0x0000000000ff0000ULL) >> 16); - buffer[4] = (uint8_t)((value & 0x000000000000ff00ULL) >> 8); - buffer[5] = (uint8_t)(value & 0x00000000000000ffULL); + buffer[0] = (uint8_t)((value >> 40) & 0xFF); + buffer[1] = (uint8_t)((value >> 32) & 0xFF); + buffer[2] = (uint8_t)((value >> 24) & 0xFF); + buffer[3] = (uint8_t)((value >> 16) & 0xFF); + buffer[4] = (uint8_t)((value >> 8) & 0xFF); + buffer[5] = (uint8_t)((value >> 0) & 0xFF); } return 6; @@ -151,17 +147,12 @@ int encode_unsigned48(uint8_t *buffer, uint64_t value) int decode_unsigned48(uint8_t *buffer, uint64_t *value) { if (buffer && value) { - *value = - ((uint64_t)((((uint64_t)buffer[0]) << 40) & 0x0000ff0000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[1]) << 32) & 0x000000ff00000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[2]) << 24) & 0x00000000ff000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[3]) << 16) & 0x0000000000ff0000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[4]) << 8) & 0x000000000000ff00ULL)); - *value |= ((uint64_t)(((uint64_t)buffer[5]) & 0x00000000000000ffULL)); + *value = (uint64_t)buffer[0] << 40; + *value |= (uint64_t)buffer[1] << 32; + *value |= (uint64_t)buffer[2] << 24; + *value |= (uint64_t)buffer[3] << 16; + *value |= (uint64_t)buffer[4] << 8; + *value |= (uint64_t)buffer[5]; } return 6; @@ -176,13 +167,13 @@ int decode_unsigned48(uint8_t *buffer, uint64_t *value) int encode_unsigned56(uint8_t *buffer, uint64_t value) { if (buffer) { - buffer[0] = (uint8_t)((value & 0x00ff000000000000ULL) >> 48); - buffer[1] = (uint8_t)((value & 0x0000ff0000000000ULL) >> 40); - buffer[2] = (uint8_t)((value & 0x000000ff00000000ULL) >> 32); - buffer[3] = (uint8_t)((value & 0x00000000ff000000ULL) >> 24); - buffer[4] = (uint8_t)((value & 0x0000000000ff0000ULL) >> 16); - buffer[5] = (uint8_t)((value & 0x000000000000ff00ULL) >> 8); - buffer[6] = (uint8_t)(value & 0x00000000000000ffULL); + buffer[0] = (uint8_t)((value >> 48) & 0xFF); + buffer[1] = (uint8_t)((value >> 40) & 0xFF); + buffer[2] = (uint8_t)((value >> 32) & 0xFF); + buffer[3] = (uint8_t)((value >> 24) & 0xFF); + buffer[4] = (uint8_t)((value >> 16) & 0xFF); + buffer[5] = (uint8_t)((value >> 8) & 0xFF); + buffer[6] = (uint8_t)((value >> 0) & 0xFF); } return 7; @@ -197,19 +188,13 @@ int encode_unsigned56(uint8_t *buffer, uint64_t value) int decode_unsigned56(uint8_t *buffer, uint64_t *value) { if (buffer && value) { - *value = - ((uint64_t)((((uint64_t)buffer[0]) << 48) & 0x00ff000000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[1]) << 40) & 0x0000ff0000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[2]) << 32) & 0x000000ff00000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[3]) << 24) & 0x00000000ff000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[4]) << 16) & 0x0000000000ff0000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[5]) << 8) & 0x000000000000ff00ULL)); - *value |= ((uint64_t)(((uint64_t)buffer[6]) & 0x00000000000000ffULL)); + *value = (uint64_t)buffer[0] << 48; + *value |= (uint64_t)buffer[1] << 40; + *value |= (uint64_t)buffer[2] << 32; + *value |= (uint64_t)buffer[3] << 24; + *value |= (uint64_t)buffer[4] << 16; + *value |= (uint64_t)buffer[5] << 8; + *value |= (uint64_t)buffer[6]; } return 7; @@ -224,14 +209,14 @@ int decode_unsigned56(uint8_t *buffer, uint64_t *value) int encode_unsigned64(uint8_t *buffer, uint64_t value) { if (buffer) { - buffer[0] = (uint8_t)((value & 0xff00000000000000ULL) >> 56); - buffer[1] = (uint8_t)((value & 0x00ff000000000000ULL) >> 48); - buffer[2] = (uint8_t)((value & 0x0000ff0000000000ULL) >> 40); - buffer[3] = (uint8_t)((value & 0x000000ff00000000ULL) >> 32); - buffer[4] = (uint8_t)((value & 0x00000000ff000000ULL) >> 24); - buffer[5] = (uint8_t)((value & 0x0000000000ff0000ULL) >> 16); - buffer[6] = (uint8_t)((value & 0x000000000000ff00ULL) >> 8); - buffer[7] = (uint8_t)(value & 0x00000000000000ffULL); + buffer[0] = (uint8_t)((value >> 56) & 0xFF); + buffer[1] = (uint8_t)((value >> 48) & 0xFF); + buffer[2] = (uint8_t)((value >> 40) & 0xFF); + buffer[3] = (uint8_t)((value >> 32) & 0xFF); + buffer[4] = (uint8_t)((value >> 24) & 0xFF); + buffer[5] = (uint8_t)((value >> 16) & 0xFF); + buffer[6] = (uint8_t)((value >> 8) & 0xFF); + buffer[7] = (uint8_t)((value >> 0) & 0xFF); } return 8; @@ -246,21 +231,14 @@ int encode_unsigned64(uint8_t *buffer, uint64_t value) int decode_unsigned64(uint8_t *buffer, uint64_t *value) { if (buffer && value) { - *value = - ((uint64_t)((((uint64_t)buffer[0]) << 56) & 0xff00000000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[1]) << 48) & 0x00ff000000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[2]) << 40) & 0x0000ff0000000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[3]) << 32) & 0x000000ff00000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[4]) << 24) & 0x00000000ff000000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[5]) << 16) & 0x0000000000ff0000ULL)); - *value |= - ((uint64_t)((((uint64_t)buffer[6]) << 8) & 0x000000000000ff00ULL)); - *value |= ((uint64_t)(((uint64_t)buffer[7]) & 0x00000000000000ffULL)); + *value = (uint64_t)buffer[0] << 56; + *value |= (uint64_t)buffer[1] << 48; + *value |= (uint64_t)buffer[2] << 40; + *value |= (uint64_t)buffer[3] << 32; + *value |= (uint64_t)buffer[4] << 24; + *value |= (uint64_t)buffer[5] << 16; + *value |= (uint64_t)buffer[6] << 8; + *value |= (uint64_t)buffer[7]; } return 8; @@ -284,13 +262,16 @@ int bacnet_unsigned_length(BACNET_UNSIGNED_INTEGER value) len = 3; } else { #ifdef UINT64_MAX - if (value <= 0x00000000FFFFFFFFULL) { + /* Avoid ULL to be compatible with C89. */ + value = value >> 32; + + if (value == 0) { len = 4; - } else if (value <= 0x000000FFFFFFFFFFULL) { + } else if (value <= 0xFF) { len = 5; - } else if (value <= 0x0000FFFFFFFFFFFFULL) { + } else if (value <= 0xFFFF) { len = 6; - } else if (value <= 0x00FFFFFFFFFFFFFFULL) { + } else if (value <= 0xFFFFFF) { len = 7; } else { len = 8; @@ -322,7 +303,7 @@ int decode_signed8(uint8_t *apdu, int32_t *value) } else { *value = 0; } - *value |= ((int32_t)(((int32_t)apdu[0]) & 0x000000ff)); + *value |= (int32_t)apdu[0] & 0xFF; } return 1; @@ -331,8 +312,8 @@ int decode_signed8(uint8_t *apdu, int32_t *value) int encode_signed16(uint8_t *apdu, int16_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff00) >> 8); - apdu[1] = (uint8_t)(value & 0x00ff); + apdu[0] = (uint8_t)((value >> 8) & 0xFF); + apdu[1] = (uint8_t)((value >> 0) & 0xFF); } return 2; @@ -347,8 +328,8 @@ int decode_signed16(uint8_t *apdu, int32_t *value) } else { *value = 0; } - *value |= ((int32_t)((((int32_t)apdu[0]) << 8) & 0x0000ff00)); - *value |= ((int32_t)(((int32_t)apdu[1]) & 0x000000ff)); + *value |= (int32_t)apdu[0] << 8; + *value |= (int32_t)apdu[1]; } return 2; @@ -357,9 +338,9 @@ int decode_signed16(uint8_t *apdu, int32_t *value) int encode_signed24(uint8_t *apdu, int32_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff0000) >> 16); - apdu[1] = (uint8_t)((value & 0x00ff00) >> 8); - apdu[2] = (uint8_t)(value & 0x0000ff); + apdu[0] = (uint8_t)((value >> 16) & 0xFF); + apdu[1] = (uint8_t)((value >> 8) & 0xFF); + apdu[2] = (uint8_t)((value >> 0) & 0xFF); } return 3; @@ -374,9 +355,9 @@ int decode_signed24(uint8_t *apdu, int32_t *value) } else { *value = 0; } - *value |= ((int32_t)((((int32_t)apdu[0]) << 16) & 0x00ff0000)); - *value |= ((int32_t)((((int32_t)apdu[1]) << 8) & 0x0000ff00)); - *value |= ((int32_t)(((int32_t)apdu[2]) & 0x000000ff)); + *value |= (int32_t)apdu[0] << 16; + *value |= (int32_t)apdu[1] << 8; + *value |= (int32_t)apdu[2]; } return 3; @@ -385,10 +366,10 @@ int decode_signed24(uint8_t *apdu, int32_t *value) int encode_signed32(uint8_t *apdu, int32_t value) { if (apdu) { - apdu[0] = (uint8_t)((value & 0xff000000) >> 24); - apdu[1] = (uint8_t)((value & 0x00ff0000) >> 16); - apdu[2] = (uint8_t)((value & 0x0000ff00) >> 8); - apdu[3] = (uint8_t)(value & 0x000000ff); + apdu[0] = (uint8_t)((value >> 24) & 0xFF); + apdu[1] = (uint8_t)((value >> 16) & 0xFF); + apdu[2] = (uint8_t)((value >> 8) & 0xFF); + apdu[3] = (uint8_t)((value >> 0) & 0xFF); } return 4; @@ -397,10 +378,10 @@ int encode_signed32(uint8_t *apdu, int32_t value) int decode_signed32(uint8_t *apdu, int32_t *value) { if (apdu && value) { - *value = ((int32_t)((((int32_t)apdu[0]) << 24) & 0xff000000)); - *value |= ((int32_t)((((int32_t)apdu[1]) << 16) & 0x00ff0000)); - *value |= ((int32_t)((((int32_t)apdu[2]) << 8) & 0x0000ff00)); - *value |= ((int32_t)(((int32_t)apdu[3]) & 0x000000ff)); + *value = (int32_t)apdu[0] << 24; + *value |= (int32_t)apdu[1] << 16; + *value |= (int32_t)apdu[2] << 8; + *value |= (int32_t)apdu[3]; } return 4; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 509ea71b..861160d5 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -10,6 +10,9 @@ if(BACNET_STACK_DEPRECATED_DISABLE) add_definitions(-DBACNET_STACK_DEPRECATED_DISABLE) endif() +# In tests allow newer C standard than in the library. +set(CMAKE_C_STANDARD 99) + # Set the compiler options if (CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_C_COMPILER_ID MATCHES "GNU") add_compile_options(-g -O0 -fprofile-arcs -ftest-coverage)