Ack Thanks Canh
-----Original Message----- From: Vu Minh Nguyen [mailto:[email protected]] Sent: Thursday, October 26, 2017 3:45 PM To: [email protected]; [email protected] Cc: [email protected]; Vu Minh Nguyen <[email protected]> Subject: [PATCH 1/1] log: fix invalid write reported by valgrind [#2657] The global pointer genLogrecord.logBuffer->logBuf was moved to local data in test case `logtest 2 39`. Therefore, any write to that global pointer results in "invalid write". --- src/log/apitest/logtest.c | 6 ++++++ src/log/apitest/tet_cfg_destination.c | 4 ++-- src/log/apitest/tet_saLogWriteLogAsync.c | 16 +++++++++++++--- src/log/apitest/tet_saLogWriteLogCallbackT.c | 4 ++-- 4 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/log/apitest/logtest.c b/src/log/apitest/logtest.c index c95c965..02d5d7d 100644 --- a/src/log/apitest/logtest.c +++ b/src/log/apitest/logtest.c @@ -77,6 +77,12 @@ static SaLogBufferT genLogBuffer = { SaNameT logSvcUsrName; +/* + * NOTE: Don't move this global pointer genLogrecord.logBuffer->logBuf. + * Do so could have impact on other test cases using this global data. + * e.g: if you move logBuffer->logBuf to your local data, writing to + * this memory in other test case will get "invalid write". + */ SaLogRecordT genLogRecord = { .logTimeStamp = SA_TIME_UNKNOWN, .logHdrType = SA_LOG_GENERIC_HEADER, diff --git a/src/log/apitest/tet_cfg_destination.c b/src/log/apitest/tet_cfg_destination.c index ba853ae..f1fe716 100644 --- a/src/log/apitest/tet_cfg_destination.c +++ b/src/log/apitest/tet_cfg_destination.c @@ -304,7 +304,7 @@ void writeToDest(void) int rc; char command[1000]; bool disable_stdout = true; - SaConstStringT s_stdout = "1> /dev/null"; + SaConstStringT s_stdout = " > /dev/null 2>&1"; if (is_executed_on_active_node() == false) { fprintf( @@ -382,7 +382,7 @@ void writeToNoDestName(void) int rc; char command[1000]; bool disable_stdout = true; - SaConstStringT s_stdout = "1> /dev/null"; + SaConstStringT s_stdout = " > /dev/null 2>&1"; if (is_executed_on_active_node() == false) { fprintf( diff --git a/src/log/apitest/tet_saLogWriteLogAsync.c b/src/log/apitest/tet_saLogWriteLogAsync.c index 2de4a23..d5586d4 100644 --- a/src/log/apitest/tet_saLogWriteLogAsync.c +++ b/src/log/apitest/tet_saLogWriteLogAsync.c @@ -420,11 +420,21 @@ void saLogWriteLogAsync_19(void) { SaInvocationT invocation = 0; char logBuf[SA_LOG_MAX_RECORD_SIZE + 10]; + SaLogBufferT logBuffer = { + .logBuf = (SaUint8T *)logBuf, + .logBufSize = sizeof(logBuf), + }; + + SaLogRecordT logRecord = { + .logTimeStamp = SA_TIME_UNKNOWN, + .logHdrType = SA_LOG_GENERIC_HEADER, + .logHeader.genericHdr.notificationClassId = NULL, + .logHeader.genericHdr.logSeverity = SA_LOG_SEV_INFO, + .logHeader.genericHdr.logSvcUsrName = &logSvcUsrName, + .logBuffer = &logBuffer}; memset(logBuf, 'A', sizeof(logBuf)); logBuf[sizeof(logBuf) - 1] = '\0'; - genLogRecord.logBuffer->logBuf = (SaUint8T *)&logBuf; - genLogRecord.logBuffer->logBufSize = SA_LOG_MAX_RECORD_SIZE + 10; rc = logInitialize(); if (rc != SA_AIS_OK) { @@ -437,7 +447,7 @@ void saLogWriteLogAsync_19(void) test_validate(rc, SA_AIS_OK); goto done; } - rc = saLogWriteLogAsync(logStreamHandle, invocation, 0, &genLogRecord); + rc = saLogWriteLogAsync(logStreamHandle, invocation, 0, &logRecord); test_validate(rc, SA_AIS_ERR_INVALID_PARAM); done: diff --git a/src/log/apitest/tet_saLogWriteLogCallbackT.c b/src/log/apitest/tet_saLogWriteLogCallbackT.c index b43af75..7385072 100644 --- a/src/log/apitest/tet_saLogWriteLogCallbackT.c +++ b/src/log/apitest/tet_saLogWriteLogCallbackT.c @@ -79,7 +79,7 @@ void saLogWriteLogCallbackT_01(void) goto done; } strcpy((char *)genLogRecord.logBuffer->logBuf, __FUNCTION__); - genLogRecord.logBuffer->logBufSize = strlen(__FUNCTION__); + genLogRecord.logBuffer->logBufSize = strlen(__FUNCTION__) + 1; struct timespec timeout_time; osaf_set_millis_timeout(2 * kWaitTime, &timeout_time); @@ -153,7 +153,7 @@ void saLogWriteLogCallbackT_02(void) } strcpy((char *)genLogRecord.logBuffer->logBuf, __FUNCTION__); - genLogRecord.logBuffer->logBufSize = strlen(__FUNCTION__); + genLogRecord.logBuffer->logBufSize = strlen(__FUNCTION__) + 1; struct timespec timeout_time; osaf_set_millis_timeout(2 * kWaitTime, &timeout_time); -- 1.9.1 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Opensaf-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensaf-devel
