You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by xiaozhongwang <gi...@git.apache.org> on 2018/01/09 05:14:30 UTC
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
GitHub user xiaozhongwang opened a pull request:
https://github.com/apache/trafodion/pull/1380
[TRAFODION-2886] fix the nullpointer error scanned by TScanCode
I scanned code of Trafodion by TScanCode, found many nullpointer error, and I fixed them here.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xiaozhongwang/trafodion TRAFODION-2886
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1380.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1380
----
commit b32351ff93e44a346f8dfb15c0ea9afa748f62a6
Author: Kenny <xi...@...>
Date: 2018-01-09T05:06:30Z
fix the nullpointer error scaned by TScanCode
commit 8866368dda821fb26107b3ed439375c8c7b23518
Author: Kenny <xi...@...>
Date: 2018-01-09T05:08:28Z
remove the error file
----
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1380
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by xiaozhongwang <gi...@git.apache.org>.
Github user xiaozhongwang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r161255440
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
--- End diff --
I cann't understand.
Why remove the || *rc,
I think there are two type results：
1、 First, if there are no memory, next_ will get NULL value.
2、 Second, there are some wrong happened in Cluster::Cluster
In the second case, next_ will get a value, but *rc will is not EXE_OK.
If we remove the || *rc, this check will pass, but there was an error happened.
My understanding is wrong?
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r161252479
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
--- End diff --
Oops. Please remove the || *rc,
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r160866708
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
- if ( ! rc ) * rc = EXE_NO_MEM_TO_EXEC;
+ * rc = EXE_NO_MEM_TO_EXEC;
--- End diff --
The deleted code is correct. In the new code rc could be NULL
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by xiaozhongwang <gi...@git.apache.org>.
Github user xiaozhongwang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r161273985
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
--- End diff --
OK, I see.
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by xiaozhongwang <gi...@git.apache.org>.
Github user xiaozhongwang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r160868578
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
- if ( ! rc ) * rc = EXE_NO_MEM_TO_EXEC;
+ * rc = EXE_NO_MEM_TO_EXEC;
--- End diff --
if rc is NULL, core at line 2397
so rc must be not NULL.
or
make sure rc is not NULL at line 2397
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r160870046
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
- if ( ! rc ) * rc = EXE_NO_MEM_TO_EXEC;
+ * rc = EXE_NO_MEM_TO_EXEC;
--- End diff --
Then the condition should be
if (!next_ || rc)
if ( !rc ) *rc = EXE_NO_MEM_TO_EXEC;
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r161272011
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
--- End diff --
You are correct, In that case you can change it if *(!next) || (*rc != EXE_OK)). I wish we got this right this time.
---
[GitHub] trafodion pull request #1380: [TRAFODION-2886] fix the nullpointer error sca...
Posted by xiaozhongwang <gi...@git.apache.org>.
Github user xiaozhongwang commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1380#discussion_r160870920
--- Diff: core/sql/executor/cluster.cpp ---
@@ -2395,7 +2395,7 @@ NABoolean Cluster::checkAndSplit(ExeErrorCode * rc)
rc);
if ( !next_ || *rc ) {
- if ( ! rc ) * rc = EXE_NO_MEM_TO_EXEC;
+ * rc = EXE_NO_MEM_TO_EXEC;
--- End diff --
the last code is:
if (!next_ || rc)
if ( NULL != rc ) *rc = EXE_NO_MEM_TO_EXEC;
the next line is wrong, because the condition !rc is TRUE when rc = NULL, so the *rc is incorrect:
if ( !rc ) *rc = EXE_NO_MEM_TO_EXEC;
---