static analysis: unintitialized struct 97/24197/8
authorMats Wichmann <mats@linux.com>
Sat, 17 Feb 2018 00:23:50 +0000 (17:23 -0700)
committerMats Wichmann <mats@linux.com>
Sat, 24 Mar 2018 13:37:26 +0000 (13:37 +0000)
A couple of instances in this file do not initialize a struct,
while the majority do so using memset. Aligned so this source file
does things consistently. Two of these (lines 709 and 748 in the
original source) are causing Coverity static checker complaints
("uninitialized scalar variable") because the struct is assigned -
even though the member in question is never actually accessed.

Note according to my understanding, for c++ code, it is considered
better to use object initialization, although if the data is
only POD (Plain Old Data), the result does come out the same.
Thus, instead of:

    CAInfo_t responseData;
    memset(&responseData, 0, sizeof(CAInfo_t));

it is suggested to use:

    CAInfo_t responseData {};

we could adjust that in a separate change if people agree.

It should not actually be necessary to pre-fill the whole struct
with zeroes since the fields that matter get values in the tests
before use anyway. However the problem with CAInfo_t is that
someone thought it was a sensible idea to have a conditional
field in an important (public API) struct in cacommon.h:

typedef struct
{
    CAMessageType_t type;       /**< Qos for the request */
.#ifdef ROUTING_GATEWAY
    bool skipRetransmission;    /**< Will not attempt retransmission even if type is CONFIRM.
                                     Required for packet forwarding */
.#endif
    uint16_t messageId;
    ...
} CAInfo_t;

which means every valid use of CAInfo_t has to have the same #ifdef
ROUTING_GATEWAY bracketing to deal with this optional member. This
is absolutely horrid in a public API and should never happen -
it means the API is different if you use different compile flags.

Change-Id: If178c7d245142e166fa33447cd00946cff4a56fc
Signed-off-by: Mats Wichmann <mats@linux.com>
resource/csdk/connectivity/test/cablocktransfertest.cpp

index d36c03d..d8d5f40 100644 (file)
@@ -697,6 +697,7 @@ TEST_F(CABlockTransferTests, CAUpdatePayloadToCADataWithRequest)
     CAGenerateToken(&tempToken, CA_MAX_TOKEN_LEN);
 
     CAInfo_t requestData;
+    memset(&requestData, 0, sizeof(CAInfo_t));
     requestData.type = CA_MSG_NONCONFIRM;
     requestData.token = tempToken;
     requestData.tokenLength = CA_MAX_TOKEN_LEN;
@@ -705,11 +706,13 @@ TEST_F(CABlockTransferTests, CAUpdatePayloadToCADataWithRequest)
     requestData.payloadSize = 0;
 
     CARequestInfo_t requestInfo;
+    memset(&requestInfo, 0, sizeof(CARequestInfo_t));
     requestInfo.method = CA_GET;
     requestInfo.info = requestData;
     requestInfo.isMulticast = false;
 
     CAData_t cadata;
+    memset(&cadata, 0, sizeof(CAData_t));
     cadata.type = SEND_TYPE_UNICAST;
     cadata.remoteEndpoint = tempRep;
     cadata.requestInfo = &requestInfo;
@@ -736,6 +739,7 @@ TEST_F(CABlockTransferTests, CAUpdatePayloadToCADataWithResponse)
     CAGenerateToken(&tempToken, CA_MAX_TOKEN_LEN);
 
     CAInfo_t responseData;
+    memset(&responseData, 0, sizeof(CAInfo_t));
     responseData.type = CA_MSG_NONCONFIRM;
     responseData.token = tempToken;
     responseData.tokenLength = CA_MAX_TOKEN_LEN;
@@ -744,10 +748,12 @@ TEST_F(CABlockTransferTests, CAUpdatePayloadToCADataWithResponse)
     responseData.payloadSize = 0;
 
     CAResponseInfo_t responseInfo;
+    memset(&responseInfo, 0, sizeof(CAResponseInfo_t));
     responseInfo.result = CA_VALID;
     responseInfo.info = responseData;
 
     CAData_t cadata;
+    memset(&cadata, 0, sizeof(CAData_t));
     cadata.type = SEND_TYPE_UNICAST;
     cadata.remoteEndpoint = tempRep;
     cadata.responseInfo = &responseInfo;