-
Notifications
You must be signed in to change notification settings - Fork 154
[TRAFODION-3039] SendEventMsg is used in a wrong way #1534
Conversation
Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/2581/ |
Test Passed. https://jenkins.esgyn.com/job/Check-PR-master/2581/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 Looks good to me.
@@ -6533,6 +6533,8 @@ odbc_SQLSrvr_ExtractLob_sme_( | |||
if (retcode == SQL_ERROR) | |||
{ | |||
ERROR_DESC_def *p_buffer = QryLobExtractSrvrStmt->sqlError.errorList._buffer; | |||
char errNumStr[128]; | |||
sprintf(errNumStr, "%d", p_buffer->sqlcode); | |||
strncpy(RequestError, p_buffer->errorText, sizeof(RequestError) - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Surround code issue. RequestError may not be null terminated when the RequestError size is less than the length of the string in p_buffer->errorText. Also, this can cause core dump due to segment violation if length of errorText is less than the size of RequestBuffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand you question, do you think RequestError is too small?
Here used strncpy, the error message may be cut, I don't think it can cause core dump.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@selvaganesang since RequestError is initialized at line 6489 and 6585, is there still a concern?
char RequestError[200] = {0};
@xiaozhongwang if you want you could also take care of line 5254 ?
Also do check regarding %d vs. %ld for sqlcode.
Sorry for the delay in review. Thanks for the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't look for this fix a long time. the branch have been deleted.
So I pull a new request for this, modified as your suggestion.
SendEventMsg use dynamic parameter.
And there is a parameter control the total, all of dynamic parameters are string data type.
But some place input a number parameter for it.
I checked it again and found two such mistakes.