From c5defe95b12e00dc0f307db4a010f03159f5aaa1 Mon Sep 17 00:00:00 2001 From: Michael Geramb Date: Wed, 19 Jun 2024 18:00:42 +0200 Subject: [PATCH 1/3] String \0 terminated in group objects --- src/knx/dptconvert.cpp | 8 +++----- src/knx/group_object.cpp | 16 +++++++++++++--- src/knx/group_object.h | 7 ++++++- src/knx/group_object_table_object.cpp | 4 ++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/knx/dptconvert.cpp b/src/knx/dptconvert.cpp index 1954d2d4..9f05fb2f 100644 --- a/src/knx/dptconvert.cpp +++ b/src/knx/dptconvert.cpp @@ -517,15 +517,13 @@ int busValueToAccess(const uint8_t* payload, size_t payload_length, const Dpt& d int busValueToString(const uint8_t* payload, size_t payload_length, const Dpt& datatype, KNXValue& value) { ASSERT_PAYLOAD(14); - char strValue[15]; - strValue[14] = '\0'; for (int n = 0; n < 14; ++n) { - strValue[n] = signed8FromPayload(payload, n); - if (!datatype.subGroup && (strValue[n] & 0x80)) + auto value = signed8FromPayload(payload, n); + if (!datatype.subGroup && (value & 0x80)) return false; } - value = strValue; + value = (const char*) payload; return true; } diff --git a/src/knx/group_object.cpp b/src/knx/group_object.cpp index 5a9ad282..844fa18a 100644 --- a/src/knx/group_object.cpp +++ b/src/knx/group_object.cpp @@ -22,7 +22,7 @@ GroupObject::GroupObject() GroupObject::GroupObject(const GroupObject& other) { - _data = new uint8_t[other._dataLength]; + _data = new uint8_t[other._table != nullptr ? other.sizeInMemory() : other._dataLength]; _commFlagEx = other._commFlagEx; _dataLength = other._dataLength; _asap = other._asap; @@ -114,12 +114,12 @@ size_t GroupObject::goSize() size_t size = sizeInTelegram(); if (size == 0) return 1; - + return size; } // see knxspec 3.5.1 p. 178 -size_t GroupObject::asapValueSize(uint8_t code) +size_t GroupObject::asapValueSize(uint8_t code) const { if (code < 7) return 0; @@ -194,6 +194,16 @@ size_t GroupObject::sizeInTelegram() return asapValueSize(code); } +size_t GroupObject::sizeInMemory() const +{ + uint8_t code = lowByte(ntohs(_table->_tableData[_asap])); + size_t result = asapValueSize(code); + if (code == 0) + return 1; + if (code == 14) + return 14 + 1; +} + #ifdef SMALL_GROUPOBJECT GroupObjectUpdatedHandler GroupObject::classCallback() { diff --git a/src/knx/group_object.h b/src/knx/group_object.h index 02bd0c37..1644e142 100644 --- a/src/knx/group_object.h +++ b/src/knx/group_object.h @@ -139,6 +139,11 @@ class GroupObject * will return 0. */ size_t sizeInTelegram(); + /** + * returns the size of the group object in the heap memory of the group object. The function returns the same value as goSize(), + * exept fot the 14 byte string type to reserve one byte of a \0 terminator character. + */ + size_t sizeInMemory() const; /** * returns the pointer to the value of the group object. This can be used if a datapoint type is not supported or if you want do * your own conversion. @@ -274,7 +279,7 @@ class GroupObject static GroupObjectUpdatedHandler _updateHandlerStatic; #endif - size_t asapValueSize(uint8_t code); + size_t asapValueSize(uint8_t code) const; size_t goSize(); uint16_t _asap = 0; ComFlagEx _commFlagEx; diff --git a/src/knx/group_object_table_object.cpp b/src/knx/group_object_table_object.cpp index bdcf8ddd..3e227e94 100644 --- a/src/knx/group_object_table_object.cpp +++ b/src/knx/group_object_table_object.cpp @@ -107,9 +107,9 @@ bool GroupObjectTableObject::initGroupObjects() GroupObject& go = _groupObjects[asap - 1]; go._asap = asap; go._table = this; - + go._dataLength = go.goSize(); - go._data = new uint8_t[go._dataLength]; + go._data = new uint8_t[go.sizeInMemory()]; memset(go._data, 0, go._dataLength); if (go.valueReadOnInit()) From ed871761525b44acbde669bb28655fbcde03aa24 Mon Sep 17 00:00:00 2001 From: Michael Geramb Date: Thu, 20 Jun 2024 21:14:30 +0200 Subject: [PATCH 2/3] Remove copy constructor, fix bugs in setting buffer to 0 --- src/knx/group_object.cpp | 25 ++++++++++++++----------- src/knx/group_object.h | 5 ++++- src/knx/group_object_table_object.cpp | 5 +++-- 3 files changed, 21 insertions(+), 14 deletions(-) diff --git a/src/knx/group_object.cpp b/src/knx/group_object.cpp index 844fa18a..e19f0c8f 100644 --- a/src/knx/group_object.cpp +++ b/src/knx/group_object.cpp @@ -20,17 +20,19 @@ GroupObject::GroupObject() #endif } -GroupObject::GroupObject(const GroupObject& other) -{ - _data = new uint8_t[other._table != nullptr ? other.sizeInMemory() : other._dataLength]; - _commFlagEx = other._commFlagEx; - _dataLength = other._dataLength; - _asap = other._asap; -#ifndef SMALL_GROUPOBJECT - _updateHandler = other._updateHandler; -#endif - memcpy(_data, other._data, _dataLength); -} +// getting the sizeInMemory requires the _table object. For this reason, the copy constructor should not be used. +// +// GroupObject::GroupObject(const GroupObject& other) +// { +// _data = new uint8_t[other._table != nullptr ? other.sizeInMemory() : other._dataLength]; +// _commFlagEx = other._commFlagEx; +// _dataLength = other._dataLength; +// _asap = other._asap; +// #ifndef SMALL_GROUPOBJECT +// _updateHandler = other._updateHandler; +// #endif +// memcpy(_data, other._data, _dataLength); +// } GroupObject::~GroupObject() { @@ -202,6 +204,7 @@ size_t GroupObject::sizeInMemory() const return 1; if (code == 14) return 14 + 1; + return result; } #ifdef SMALL_GROUPOBJECT diff --git a/src/knx/group_object.h b/src/knx/group_object.h index 1644e142..31449da8 100644 --- a/src/knx/group_object.h +++ b/src/knx/group_object.h @@ -60,7 +60,10 @@ class GroupObject /** * The copy constructor. */ - GroupObject(const GroupObject& other); + + // getting the sizeInMemory requires the _table object. For this reason, the copy constructor should not be used + // GroupObject(const GroupObject& other); + /** * The destructor. */ diff --git a/src/knx/group_object_table_object.cpp b/src/knx/group_object_table_object.cpp index 3e227e94..8d931ced 100644 --- a/src/knx/group_object_table_object.cpp +++ b/src/knx/group_object_table_object.cpp @@ -109,8 +109,9 @@ bool GroupObjectTableObject::initGroupObjects() go._table = this; go._dataLength = go.goSize(); - go._data = new uint8_t[go.sizeInMemory()]; - memset(go._data, 0, go._dataLength); + size_t sizeInMemory = go.sizeInMemory(); + go._data = new uint8_t[sizeInMemory]; + memset(go._data, 0, sizeInMemory); if (go.valueReadOnInit()) go.requestObjectRead(); From 80da75f80abfba1b6b5eb032d2334ae9cbfb678b Mon Sep 17 00:00:00 2001 From: Michael Geramb Date: Sat, 20 Jul 2024 14:52:15 +0200 Subject: [PATCH 3/3] Remove copy constructor in GroupObject --- src/knx/group_object.cpp | 14 -------------- src/knx/group_object.h | 7 ------- 2 files changed, 21 deletions(-) diff --git a/src/knx/group_object.cpp b/src/knx/group_object.cpp index e19f0c8f..7b172d5c 100644 --- a/src/knx/group_object.cpp +++ b/src/knx/group_object.cpp @@ -20,20 +20,6 @@ GroupObject::GroupObject() #endif } -// getting the sizeInMemory requires the _table object. For this reason, the copy constructor should not be used. -// -// GroupObject::GroupObject(const GroupObject& other) -// { -// _data = new uint8_t[other._table != nullptr ? other.sizeInMemory() : other._dataLength]; -// _commFlagEx = other._commFlagEx; -// _dataLength = other._dataLength; -// _asap = other._asap; -// #ifndef SMALL_GROUPOBJECT -// _updateHandler = other._updateHandler; -// #endif -// memcpy(_data, other._data, _dataLength); -// } - GroupObject::~GroupObject() { if (_data) diff --git a/src/knx/group_object.h b/src/knx/group_object.h index 31449da8..244fab67 100644 --- a/src/knx/group_object.h +++ b/src/knx/group_object.h @@ -57,13 +57,6 @@ class GroupObject * The constructor. */ GroupObject(); - /** - * The copy constructor. - */ - - // getting the sizeInMemory requires the _table object. For this reason, the copy constructor should not be used - // GroupObject(const GroupObject& other); - /** * The destructor. */