From 118a731b362a9458ac35dc9479cb440116e375ad Mon Sep 17 00:00:00 2001 From: Michael Zillgith Date: Sat, 4 Jan 2020 17:40:26 +0100 Subject: [PATCH] - SV publisher: fixed memory leaks (#191) --- examples/sv_publisher/sv_publisher_example.c | 2 + hal/ethernet/linux/ethernet_linux.c | 48 +++++------ src/sampled_values/sv_publisher.c | 87 ++++++++++++-------- 3 files changed, 80 insertions(+), 57 deletions(-) diff --git a/examples/sv_publisher/sv_publisher_example.c b/examples/sv_publisher/sv_publisher_example.c index a74ab24d..4b41401c 100644 --- a/examples/sv_publisher/sv_publisher_example.c +++ b/examples/sv_publisher/sv_publisher_example.c @@ -29,6 +29,8 @@ main(int argc, char** argv) printf("Using interface %s\n", interface); + signal(SIGINT, sigint_handler); + SVPublisher svPublisher = SVPublisher_create(NULL, interface); if (svPublisher) { diff --git a/hal/ethernet/linux/ethernet_linux.c b/hal/ethernet/linux/ethernet_linux.c index 49e743cc..c3ab2e20 100644 --- a/hal/ethernet/linux/ethernet_linux.c +++ b/hal/ethernet/linux/ethernet_linux.c @@ -182,38 +182,40 @@ Ethernet_createSocket(const char* interfaceId, uint8_t* destAddress) { EthernetSocket ethernetSocket = GLOBAL_CALLOC(1, sizeof(struct sEthernetSocket)); - ethernetSocket->rawSocket = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); - - if (ethernetSocket->rawSocket == -1) { - if (DEBUG_SOCKET) - printf("Error creating raw socket!\n"); - GLOBAL_FREEMEM(ethernetSocket); - return NULL; - } + if (ethernetSocket) { + ethernetSocket->rawSocket = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL)); + + if (ethernetSocket->rawSocket == -1) { + if (DEBUG_SOCKET) + printf("Error creating raw socket!\n"); + GLOBAL_FREEMEM(ethernetSocket); + return NULL; + } - ethernetSocket->socketAddress.sll_family = PF_PACKET; - ethernetSocket->socketAddress.sll_protocol = htons(ETH_P_IP); + ethernetSocket->socketAddress.sll_family = PF_PACKET; + ethernetSocket->socketAddress.sll_protocol = htons(ETH_P_IP); - int ifcIdx = getInterfaceIndex(ethernetSocket->rawSocket, interfaceId); + int ifcIdx = getInterfaceIndex(ethernetSocket->rawSocket, interfaceId); - if (ifcIdx == -1) { - Ethernet_destroySocket(ethernetSocket); - return NULL; - } + if (ifcIdx == -1) { + Ethernet_destroySocket(ethernetSocket); + return NULL; + } - ethernetSocket->socketAddress.sll_ifindex = ifcIdx; + ethernetSocket->socketAddress.sll_ifindex = ifcIdx; - ethernetSocket->socketAddress.sll_hatype = ARPHRD_ETHER; - ethernetSocket->socketAddress.sll_pkttype = PACKET_OTHERHOST; + ethernetSocket->socketAddress.sll_hatype = ARPHRD_ETHER; + ethernetSocket->socketAddress.sll_pkttype = PACKET_OTHERHOST; - ethernetSocket->socketAddress.sll_halen = ETH_ALEN; + ethernetSocket->socketAddress.sll_halen = ETH_ALEN; - memset(ethernetSocket->socketAddress.sll_addr, 0, 8); + memset(ethernetSocket->socketAddress.sll_addr, 0, 8); - if (destAddress != NULL) - memcpy(ethernetSocket->socketAddress.sll_addr, destAddress, 6); + if (destAddress != NULL) + memcpy(ethernetSocket->socketAddress.sll_addr, destAddress, 6); - ethernetSocket->isBind = false; + ethernetSocket->isBind = false; + } return ethernetSocket; } diff --git a/src/sampled_values/sv_publisher.c b/src/sampled_values/sv_publisher.c index 90d2de7b..1f7b0110 100644 --- a/src/sampled_values/sv_publisher.c +++ b/src/sampled_values/sv_publisher.c @@ -129,48 +129,53 @@ preparePacketBuffer(SVPublisher self, CommParameters* parameters, const char* in self->buffer = (uint8_t*) GLOBAL_MALLOC(SV_MAX_MESSAGE_SIZE); - memcpy(self->buffer, dstAddr, 6); - memcpy(self->buffer + 6, srcAddr, 6); + if (self->buffer) { + memcpy(self->buffer, dstAddr, 6); + memcpy(self->buffer + 6, srcAddr, 6); - int bufPos = 12; + int bufPos = 12; - if (useVlanTags) { - /* Priority tag - IEEE 802.1Q */ - self->buffer[bufPos++] = 0x81; - self->buffer[bufPos++] = 0x00; + if (useVlanTags) { + /* Priority tag - IEEE 802.1Q */ + self->buffer[bufPos++] = 0x81; + self->buffer[bufPos++] = 0x00; - uint8_t tci1 = priority << 5; - tci1 += vlanId / 256; + uint8_t tci1 = priority << 5; + tci1 += vlanId / 256; - uint8_t tci2 = vlanId % 256; + uint8_t tci2 = vlanId % 256; - self->buffer[bufPos++] = tci1; /* Priority + VLAN-ID */ - self->buffer[bufPos++] = tci2; /* VLAN-ID */ - } + self->buffer[bufPos++] = tci1; /* Priority + VLAN-ID */ + self->buffer[bufPos++] = tci2; /* VLAN-ID */ + } - /* EtherType Sampled Values */ - self->buffer[bufPos++] = 0x88; - self->buffer[bufPos++] = 0xBa; + /* EtherType Sampled Values */ + self->buffer[bufPos++] = 0x88; + self->buffer[bufPos++] = 0xBa; - /* APPID */ - self->buffer[bufPos++] = appId / 256; - self->buffer[bufPos++] = appId % 256; + /* APPID */ + self->buffer[bufPos++] = appId / 256; + self->buffer[bufPos++] = appId % 256; - self->lengthField = bufPos; + self->lengthField = bufPos; - /* Length */ - self->buffer[bufPos++] = 0x00; - self->buffer[bufPos++] = 0x08; + /* Length */ + self->buffer[bufPos++] = 0x00; + self->buffer[bufPos++] = 0x08; - /* Reserved1 */ - self->buffer[bufPos++] = 0x00; - self->buffer[bufPos++] = 0x00; + /* Reserved1 */ + self->buffer[bufPos++] = 0x00; + self->buffer[bufPos++] = 0x00; - /* Reserved2 */ - self->buffer[bufPos++] = 0x00; - self->buffer[bufPos++] = 0x00; + /* Reserved2 */ + self->buffer[bufPos++] = 0x00; + self->buffer[bufPos++] = 0x00; - self->payloadStart = bufPos; + self->payloadStart = bufPos; + } + else { + return false; + } return true; } @@ -303,7 +308,7 @@ SVPublisher_createEx(CommParameters* parameters, const char* interfaceId, bool u self->asduList = NULL; if (preparePacketBuffer(self, parameters, interfaceId, useVlanTag) == false) { - GLOBAL_FREEMEM(self); + SVPublisher_destroy(self); self = NULL; } @@ -502,7 +507,6 @@ SVPublisher_setupComplete(SVPublisher self) } - void SVPublisher_publish(SVPublisher self) { @@ -512,11 +516,26 @@ SVPublisher_publish(SVPublisher self) Ethernet_sendPacket(self->ethernetSocket, self->buffer, self->payloadStart + self->payloadLength); } - void SVPublisher_destroy(SVPublisher self) { - GLOBAL_FREEMEM(self->buffer); + if (self->ethernetSocket) + Ethernet_destroySocket(self->ethernetSocket); + + if (self->buffer) + GLOBAL_FREEMEM(self->buffer); + + SVPublisher_ASDU asdu = self->asduList; + + while (asdu) { + SVPublisher_ASDU nextAsdu = asdu->_next; + + GLOBAL_FREEMEM(asdu); + + asdu = nextAsdu; + } + + GLOBAL_FREEMEM(self); }