Ack Thanks Lennart
> -----Original Message----- > From: Vu Minh Nguyen > Sent: den 26 oktober 2017 10:45 > To: Lennart Lund <[email protected]>; Canh Van Truong > <[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
