From 4f0667b5971f125e74814e3ed298b7921506bc02 Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Sat, 30 Jan 2021 16:46:58 +0100 Subject: [PATCH] - removed some warnings and code cleanup --- Makefile | 4 +- hal/ethernet/linux/ethernet_linux.c | 6 +-- hal/inc/hal_ethernet.h | 4 +- hal/serial/linux/serial_port_linux.c | 2 +- src/common/inc/linked_list.h | 4 +- src/common/string_utilities.c | 2 +- src/goose/goose_receiver.c | 2 +- src/iec61850/client/ied_connection.c | 50 ++++++++----------- src/iec61850/server/mms_mapping/control.c | 9 ++-- src/iec61850/server/mms_mapping/reporting.c | 2 +- src/mms/asn1/ber_encoder.c | 23 ++++----- src/mms/iso_acse/acse.c | 2 +- src/mms/iso_mms/asn1c/ber_decoder.c | 2 +- src/mms/iso_mms/common/mms_value.c | 2 +- src/mms/iso_mms/server/mms_access_result.c | 5 +- .../iso_mms/server/mms_get_namelist_service.c | 4 +- src/mms/iso_server/iso_server.c | 4 +- src/sampled_values/sv_subscriber.c | 25 +++++----- 18 files changed, 71 insertions(+), 81 deletions(-) diff --git a/Makefile b/Makefile index bfffdee7..60a798f6 100644 --- a/Makefile +++ b/Makefile @@ -122,7 +122,7 @@ LIB_SOURCES = $(call get_sources,$(LIB_SOURCE_DIRS)) LIB_OBJS = $(call src_to,.o,$(LIB_SOURCES)) CFLAGS += -std=gnu99 -#CFLAGS += -Wno-error=format +CFLAGS += -Wno-error=format CFLAGS += -Wstrict-prototypes ifneq ($(HAL_IMPL), WIN32) @@ -136,6 +136,8 @@ CFLAGS += -Wmissing-declarations CFLAGS += -Wshadow CFLAGS += -Wall CFLAGS += -Wextra +CFLAGS += -Wno-format +#CFLAGS += -Wconditional-uninitialized #CFLAGS += -Werror all: lib diff --git a/hal/ethernet/linux/ethernet_linux.c b/hal/ethernet/linux/ethernet_linux.c index 79781efe..728b5d10 100644 --- a/hal/ethernet/linux/ethernet_linux.c +++ b/hal/ethernet/linux/ethernet_linux.c @@ -48,7 +48,7 @@ struct sEthernetSocket { }; struct sEthernetHandleSet { - struct pollfd *handles; + struct pollfd* handles; int nhandles; }; @@ -125,7 +125,7 @@ getInterfaceIndex(int sock, const char* deviceName) { struct ifreq ifr; - strncpy(ifr.ifr_name, deviceName, IFNAMSIZ); + strncpy(ifr.ifr_name, deviceName, IFNAMSIZ - 1); if (ioctl(sock, SIOCGIFINDEX, &ifr) == -1) { if (DEBUG_SOCKET) @@ -163,7 +163,7 @@ Ethernet_getInterfaceMACAddress(const char* interfaceId, uint8_t* addr) memset(&buffer, 0x00, sizeof(buffer)); - strncpy(buffer.ifr_name, interfaceId, IFNAMSIZ); + strncpy(buffer.ifr_name, interfaceId, IFNAMSIZ - 1); ioctl(sock, SIOCGIFHWADDR, &buffer); diff --git a/hal/inc/hal_ethernet.h b/hal/inc/hal_ethernet.h index 7f243dce..c7822407 100644 --- a/hal/inc/hal_ethernet.h +++ b/hal/inc/hal_ethernet.h @@ -83,7 +83,7 @@ EthernetHandleSet_removeSocket(EthernetHandleSet self, const EthernetSocket sock * The function will return after \p timeoutMs ms if no data is pending. * * \param self the HandleSet instance - * \param timeout in milliseconds (ms) + * \param timeoutMs in milliseconds (ms) * \return It returns the number of sockets on which data is pending * or 0 if no data is pending on any of the monitored connections. * The function shall return -1 if a socket error occures. @@ -145,7 +145,7 @@ Ethernet_setProtocolFilter(EthernetSocket ethSocket, uint16_t etherType); * * \param ethSocket the ethernet socket handle * \param buffer the buffer to copy the message to - * \param the maximum size of the buffer + * \param bufferSize the maximum size of the buffer * * \return size of message received in bytes */ diff --git a/hal/serial/linux/serial_port_linux.c b/hal/serial/linux/serial_port_linux.c index 5cdfd085..64acf77d 100644 --- a/hal/serial/linux/serial_port_linux.c +++ b/hal/serial/linux/serial_port_linux.c @@ -62,7 +62,7 @@ SerialPort_create(const char* interfaceName, int baudRate, uint8_t dataBits, cha self->lastSentTime = 0; self->timeout.tv_sec = 0; self->timeout.tv_usec = 100000; /* 100 ms */ - strncpy(self->interfaceName, interfaceName, 100); + strncpy(self->interfaceName, interfaceName, 99); self->lastError = SERIAL_PORT_ERROR_NONE; } diff --git a/src/common/inc/linked_list.h b/src/common/inc/linked_list.h index b0054f4d..cc4b9774 100644 --- a/src/common/inc/linked_list.h +++ b/src/common/inc/linked_list.h @@ -1,7 +1,7 @@ /* * linked_list.h * - * Copyright 2013-2018 Michael Zillgith + * Copyright 2013-2021 Michael Zillgith * * This file is part of libIEC61850. * @@ -153,7 +153,7 @@ LinkedList_getNext(LinkedList self); /** * \brief Get the last element in the list. * - * \param listElement the LinkedList instance + * \param self the LinkedList instance */ LIB61850_API LinkedList LinkedList_getLastElement(LinkedList self); diff --git a/src/common/string_utilities.c b/src/common/string_utilities.c index 25aeb6f2..cb121690 100644 --- a/src/common/string_utilities.c +++ b/src/common/string_utilities.c @@ -271,7 +271,7 @@ getCharWeight(int c) { static bool initialized = false; static char lookupTable[LT_MAX_CHARS + 1]; - static char* charOrder = "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz$_0123456789"; + static const char* charOrder = "AaBbCcDdEeFfGgHhIiJjKkLlMmNnOoPpQqRrSsTtUuVvWwXxYyZz$_0123456789"; if (!initialized) { int ltIndex; diff --git a/src/goose/goose_receiver.c b/src/goose/goose_receiver.c index 00d0bff2..bed369df 100644 --- a/src/goose/goose_receiver.c +++ b/src/goose/goose_receiver.c @@ -574,7 +574,7 @@ parseGoosePayload(GooseReceiver self, uint8_t* buffer, int apduLength) uint32_t timeAllowedToLive = 0; uint32_t stNum = 0; uint32_t sqNum = 0; - uint32_t confRev; + uint32_t confRev = 0; bool simulation = false; bool ndsCom = false; GooseSubscriber matchingSubscriber = NULL; diff --git a/src/iec61850/client/ied_connection.c b/src/iec61850/client/ied_connection.c index 0e2da178..3b9b4361 100644 --- a/src/iec61850/client/ied_connection.c +++ b/src/iec61850/client/ied_connection.c @@ -1191,7 +1191,7 @@ IedConnection_readObject(IedConnection self, IedClientError* error, const char* return NULL; } - MmsError mmsError; + MmsError mmsError = MMS_ERROR_NONE; /* check if item ID contains an array "(..)" */ char* brace = strchr(itemId, '('); @@ -2482,22 +2482,20 @@ IedConnection_getLogicalNodeDirectory(IedConnection self, IedClientError* error, LinkedList device = LinkedList_getNext(self->logicalDevices); - bool deviceFound = false; - - ICLogicalDevice* ld; + ICLogicalDevice* ld = NULL; while (device != NULL) { - ld = (ICLogicalDevice*) device->data; + ICLogicalDevice* ldCandidate = (ICLogicalDevice*) device->data; - if (strcmp(logicalDeviceName, ld->name) == 0) { - deviceFound = true; + if (strcmp(logicalDeviceName, ldCandidate->name) == 0) { + ld = ldCandidate; break; } device = LinkedList_getNext(device); } - if (!deviceFound) { + if (ld == NULL) { *error = IED_ERROR_OBJECT_REFERENCE_INVALID; return NULL; } @@ -2630,22 +2628,20 @@ IedConnection_getLogicalNodeVariables(IedConnection self, IedClientError* error, LinkedList device = LinkedList_getNext(self->logicalDevices); - bool deviceFound = false; - - ICLogicalDevice* ld; + ICLogicalDevice* ld = NULL; while (device != NULL) { - ld = (ICLogicalDevice*) device->data; + ICLogicalDevice* ldCandidate = (ICLogicalDevice*) device->data; - if (strcmp(logicalDeviceName, ld->name) == 0) { - deviceFound = true; + if (strcmp(logicalDeviceName, ldCandidate->name) == 0) { + ld = ldCandidate; break; } device = LinkedList_getNext(device); } - if (!deviceFound) { + if (ld == NULL) { *error = IED_ERROR_OBJECT_REFERENCE_INVALID; return NULL; } @@ -2731,22 +2727,20 @@ getDataDirectory(IedConnection self, IedClientError* error, LinkedList device = LinkedList_getNext(self->logicalDevices); - bool deviceFound = false; - - ICLogicalDevice* ld; + ICLogicalDevice* ld = NULL; while (device != NULL) { - ld = (ICLogicalDevice*) device->data; + ICLogicalDevice* ldCandidate = (ICLogicalDevice*) device->data; - if (strcmp(logicalDeviceName, ld->name) == 0) { - deviceFound = true; + if (strcmp(logicalDeviceName, ldCandidate->name) == 0) { + ld = ldCandidate; break; } device = LinkedList_getNext(device); } - if (!deviceFound) { + if (ld == NULL) { *error = IED_ERROR_OBJECT_REFERENCE_INVALID; return NULL; } @@ -2900,22 +2894,20 @@ getDataDirectoryByFc(IedConnection self, IedClientError* error, LinkedList device = LinkedList_getNext(self->logicalDevices); - bool deviceFound = false; - - ICLogicalDevice* ld; + ICLogicalDevice* ld = NULL; while (device != NULL) { - ld = (ICLogicalDevice*) device->data; + ICLogicalDevice* ldCandidate = (ICLogicalDevice*) device->data; - if (strcmp(logicalDeviceName, ld->name) == 0) { - deviceFound = true; + if (strcmp(logicalDeviceName, ldCandidate->name) == 0) { + ld = ldCandidate; break; } device = LinkedList_getNext(device); } - if (!deviceFound) { + if (ld == NULL) { *error = IED_ERROR_OBJECT_REFERENCE_INVALID; return NULL; } diff --git a/src/iec61850/server/mms_mapping/control.c b/src/iec61850/server/mms_mapping/control.c index cb1a3d21..5bed8f1e 100644 --- a/src/iec61850/server/mms_mapping/control.c +++ b/src/iec61850/server/mms_mapping/control.c @@ -678,9 +678,11 @@ operateControl(ControlObject* self, MmsValue* value, uint64_t currentTime, bool static void resetAddCause(ControlObject* self) { - self->addCauseValue = ADD_CAUSE_UNKNOWN; + if (self) { + self->addCauseValue = ADD_CAUSE_UNKNOWN; - MmsValue_setInt32(self->addCause, self->addCauseValue); + MmsValue_setInt32(self->addCause, self->addCauseValue); + } } static void @@ -1869,6 +1871,7 @@ Control_writeAccessControlObject(MmsMapping* self, MmsDomain* domain, char* vari { MmsDataAccessError indication = DATA_ACCESS_ERROR_OBJECT_ACCESS_DENIED; IEC61850_ServiceError serviceError = IEC61850_SERVICE_ERROR_NO_ERROR; + ControlObject* controlObject = NULL; if (DEBUG_IED_SERVER) printf("IED_SERVER: writeAccessControlObject: %s\n", variableIdOrig); @@ -1935,7 +1938,7 @@ Control_writeAccessControlObject(MmsMapping* self, MmsDomain* domain, char* vari goto free_and_return; } - ControlObject* controlObject = Control_lookupControlObject(self, domain, lnName, objectName); + controlObject = Control_lookupControlObject(self, domain, lnName, objectName); if (controlObject == NULL) { indication = DATA_ACCESS_ERROR_OBJECT_ACCESS_DENIED; diff --git a/src/iec61850/server/mms_mapping/reporting.c b/src/iec61850/server/mms_mapping/reporting.c index 06d68258..40454739 100644 --- a/src/iec61850/server/mms_mapping/reporting.c +++ b/src/iec61850/server/mms_mapping/reporting.c @@ -1410,7 +1410,7 @@ convertIPv4AddressStringToByteArray(const char* clientAddressString, uint8_t ipV { int addrElementCount = 0; - char* separator = (char*) clientAddressString; + const char* separator = clientAddressString; while (separator != NULL && addrElementCount < 4) { int intVal = atoi(separator); diff --git a/src/mms/asn1/ber_encoder.c b/src/mms/asn1/ber_encoder.c index 0a63df3d..181e83a0 100644 --- a/src/mms/asn1/ber_encoder.c +++ b/src/mms/asn1/ber_encoder.c @@ -37,15 +37,15 @@ BerEncoder_encodeLength(uint32_t length, uint8_t* buffer, int bufPos) else if (length < 65535) { buffer[bufPos++] = 0x82; - buffer[bufPos++] = length / 256; - buffer[bufPos++] = length % 256; + buffer[bufPos++] = (uint8_t) (length / 256); + buffer[bufPos++] = (uint8_t) (length % 256); } else { buffer[bufPos++] = 0x83; - buffer[bufPos++] = length / 0x10000; - buffer[bufPos++] = (length & 0xffff) / 0x100; - buffer[bufPos++] = length % 256; + buffer[bufPos++] = (uint8_t) (length / 0x10000); + buffer[bufPos++] = (uint8_t) ((length & 0xffff) / 0x100); + buffer[bufPos++] = (uint8_t) (length % 256); } return bufPos; @@ -80,13 +80,13 @@ BerEncoder_encodeStringWithTag(uint8_t tag, const char* string, uint8_t* buffer, buffer[bufPos++] = tag; if (string != NULL) { - int length = strlen(string); + int length = (int) strlen(string); - bufPos = BerEncoder_encodeLength(length, buffer, bufPos); + bufPos = BerEncoder_encodeLength((uint32_t) length, buffer, bufPos); int i; for (i = 0; i < length; i++) { - buffer[bufPos++] = string[i]; + buffer[bufPos++] = (uint8_t) string[i]; } } else @@ -316,8 +316,7 @@ int BerEncoder_encodeFloat(uint8_t* floatValue, uint8_t formatWidth, uint8_t exponentWidth, uint8_t* buffer, int bufPos) { - /* TODO operate on encoding buffer directly */ - uint8_t valueBuffer[9]; + uint8_t* valueBuffer = buffer + bufPos; int byteSize = formatWidth / 8; @@ -332,9 +331,7 @@ BerEncoder_encodeFloat(uint8_t* floatValue, uint8_t formatWidth, uint8_t exponen BerEncoder_revertByteOrder(valueBuffer + 1, byteSize); #endif - for (i = 0; i < byteSize + 1; i++) { - buffer[bufPos++] = valueBuffer[i]; - } + bufPos = bufPos + 1 + byteSize; return bufPos; } diff --git a/src/mms/iso_acse/acse.c b/src/mms/iso_acse/acse.c index bbdfcb36..60b0f534 100644 --- a/src/mms/iso_acse/acse.c +++ b/src/mms/iso_acse/acse.c @@ -418,7 +418,7 @@ AcseConnection_init(AcseConnection* self, AcseAuthenticator authenticator, void* AcseIndication AcseConnection_parseMessage(AcseConnection* self, ByteBuffer* message) { - AcseIndication indication; + AcseIndication indication = ACSE_ERROR; uint8_t* buffer = message->buffer; diff --git a/src/mms/iso_mms/asn1c/ber_decoder.c b/src/mms/iso_mms/asn1c/ber_decoder.c index de0695ae..a3cfbbeb 100644 --- a/src/mms/iso_mms/asn1c/ber_decoder.c +++ b/src/mms/iso_mms/asn1c/ber_decoder.c @@ -70,7 +70,7 @@ ber_check_tags(asn_codec_ctx_t *opt_codec_ctx, ssize_t tag_len; ssize_t len_len; ber_tlv_tag_t tlv_tag; - ber_tlv_len_t tlv_len; + ber_tlv_len_t tlv_len = -1; ber_tlv_len_t limit_len = -1; int expect_00_terminators = 0; int tlv_constr = -1; /* If CHOICE, opt_tlv_form is not given */ diff --git a/src/mms/iso_mms/common/mms_value.c b/src/mms/iso_mms/common/mms_value.c index 88eda231..3e155320 100644 --- a/src/mms/iso_mms/common/mms_value.c +++ b/src/mms/iso_mms/common/mms_value.c @@ -654,7 +654,7 @@ void MmsValue_setInt16(MmsValue* self, int16_t integer) { if (self->type == MMS_INTEGER) { - if (Asn1PrimitiveValue_getMaxSize(self->value.integer) >= 1) { + if (Asn1PrimitiveValue_getMaxSize(self->value.integer) >= 2) { BerInteger_setInt32(self->value.integer, (int32_t) integer); } } diff --git a/src/mms/iso_mms/server/mms_access_result.c b/src/mms/iso_mms/server/mms_access_result.c index 4671211f..0beda10f 100644 --- a/src/mms/iso_mms/server/mms_access_result.c +++ b/src/mms/iso_mms/server/mms_access_result.c @@ -1,7 +1,7 @@ /* * mms_access_result.c * - * Copyright 2013, 2016 Michael Zillgith + * Copyright 2013-2021 Michael Zillgith * * This file is part of libIEC61850. * @@ -329,7 +329,7 @@ exit_with_error: int MmsValue_encodeMmsData(MmsValue* self, uint8_t* buffer, int bufPos, bool encode) { - int size; + int size = 0; switch (self->type) { case MMS_BOOLEAN: @@ -446,4 +446,3 @@ MmsValue_encodeMmsData(MmsValue* self, uint8_t* buffer, int bufPos, bool encode) else return size; } - diff --git a/src/mms/iso_mms/server/mms_get_namelist_service.c b/src/mms/iso_mms/server/mms_get_namelist_service.c index 7e7e187b..c9b90723 100644 --- a/src/mms/iso_mms/server/mms_get_namelist_service.c +++ b/src/mms/iso_mms/server/mms_get_namelist_service.c @@ -438,10 +438,10 @@ mmsServer_handleGetNameListRequest( int objectScope = -1; char* domainId = NULL; - int domainIdLength; + int domainIdLength = 0; char* continueAfter = NULL; - int continueAfterLength; + int continueAfterLength = 0; while (bufPos < maxBufPos) { uint8_t tag = buffer[bufPos++]; diff --git a/src/mms/iso_server/iso_server.c b/src/mms/iso_server/iso_server.c index 0ffeb5cb..e45224f9 100644 --- a/src/mms/iso_server/iso_server.c +++ b/src/mms/iso_server/iso_server.c @@ -705,7 +705,7 @@ IsoServer_startListeningThreadless(IsoServer self) int IsoServer_waitReady(IsoServer self, unsigned int timeoutMs) { - int result; + int result = -1; if (getState(self) == ISO_SVR_STATE_RUNNING) { @@ -717,8 +717,6 @@ IsoServer_waitReady(IsoServer self, unsigned int timeoutMs) printf("ISO_SERVER: internal error - no handleset!\n"); } - } else { - result = -1; } return result; diff --git a/src/sampled_values/sv_subscriber.c b/src/sampled_values/sv_subscriber.c index 6e6904df..f37b3fbc 100644 --- a/src/sampled_values/sv_subscriber.c +++ b/src/sampled_values/sv_subscriber.c @@ -22,6 +22,7 @@ */ #include "stack_config.h" +#include #include "libiec61850_platform_includes.h" @@ -355,8 +356,10 @@ parseASDU(SVReceiver self, SVSubscriber subscriber, uint8_t* buffer, int length) if (SVSubscriber_ASDU_hasDatSet(&asdu)) printf("SV_SUBSCRIBER: DatSet: %s\n", asdu.datSet); +#ifdef PRIu64 if (SVSubscriber_ASDU_hasRefrTm(&asdu)) - printf("SV_SUBSCRIBER: RefrTm[ns]: %llu\n", SVSubscriber_ASDU_getRefrTmAsNs(&asdu)); + printf("SV_SUBSCRIBER: RefrTm[ns]: %"PRIu64"\n", SVSubscriber_ASDU_getRefrTmAsNs(&asdu)); +#endif if (SVSubscriber_ASDU_hasSmpMod(&asdu)) printf("SV_SUBSCRIBER: SmpMod: %d\n", SVSubscriber_ASDU_getSmpMod(&asdu)); if (SVSubscriber_ASDU_hasSmpRate(&asdu)) @@ -455,7 +458,6 @@ static void parseSVMessage(SVReceiver self, int numbytes) { int bufPos; - bool subscriberFound = false; uint8_t* buffer = self->buffer; if (numbytes < 22) return; @@ -507,25 +509,24 @@ parseSVMessage(SVReceiver self, int numbytes) printf("SV_SUBSCRIBER: APDU length: %i\n", apduLength); } - /* check if there is a matching subscriber */ #if (CONFIG_MMS_THREADLESS_STACK == 0) Semaphore_wait(self->subscriberListLock); #endif - LinkedList element = LinkedList_getNext(self->subscriberList); + SVSubscriber subscriber = NULL; - SVSubscriber subscriber; + LinkedList element = LinkedList_getNext(self->subscriberList); while (element != NULL) { - subscriber = (SVSubscriber) LinkedList_getData(element); + SVSubscriber subscriberElem = (SVSubscriber) LinkedList_getData(element); - if (subscriber->appId == appId) { + if (subscriberElem->appId == appId) { if (self->checkDestAddr) { - if (memcmp(dstAddr, subscriber->ethAddr, 6) == 0) { - subscriberFound = true; + if (memcmp(dstAddr, subscriberElem->ethAddr, 6) == 0) { + subscriber = subscriberElem; break; } else @@ -533,11 +534,10 @@ parseSVMessage(SVReceiver self, int numbytes) printf("SV_SUBSCRIBER: Checking ethernet dest address failed!\n"); } else { - subscriberFound = true; + subscriber = subscriberElem; break; } - } element = LinkedList_getNext(element); @@ -547,8 +547,7 @@ parseSVMessage(SVReceiver self, int numbytes) Semaphore_post(self->subscriberListLock); #endif - - if (subscriberFound) + if (subscriber) parseSVPayload(self, subscriber, buffer + bufPos, apduLength); else { if (DEBUG_SV_SUBSCRIBER)