You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Jim Apple (Code Review)" <ge...@cloudera.org> on 2018/10/27 23:31:07 UTC

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior 

Jim Apple has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11811


Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................

IMPALA-5031: Zero-length arrays are undefined behavior

This patch prevents the creation of a zero-length array. This is
illegal under paragraph 5 of §6.7.5.2 ("Array declarators") of the C99
standard, which reads:

  If the size is an expression that is not an integer constant
  expression: if it occurs in a declaration at function prototype
  scope, it is treated as if it were replaced by *; otherwise, each
  time it is evaluated it shall have a value greater than zero.

Variable-length arrays are not part of C++14, but they are a common
compiler extension and are available in both clang++ and g++. That the
semantics of missing features are the same is implied by [intro.scope]
in the C++14 standard, which reads:

  C++ is a general purpose programming language based on the C
  programming language as described in ISO/IEC 9899:1999 Programming
  languages — C (hereinafter referred to as the C standard).

Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
---
M be/src/runtime/row-batch-serialize-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/11811/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1190/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Sun, 28 Oct 2018 00:05:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11811/1/be/src/runtime/row-batch-serialize-test.cc
File be/src/runtime/row-batch-serialize-test.cc:

http://gerrit.cloudera.org:8080/#/c/11811/1/be/src/runtime/row-batch-serialize-test.cc@242
PS1, Line 242:         int len = (rand() % MAX_STRING_LEN) + 1;
I think we want to generate empty strings as random values. Why not just make buf a non-VLA of MAX_STRING_LEN.



-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 1
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Nov 2018 01:11:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 3: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 23:41:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11811/1/be/src/runtime/row-batch-serialize-test.cc
File be/src/runtime/row-batch-serialize-test.cc:

http://gerrit.cloudera.org:8080/#/c/11811/1/be/src/runtime/row-batch-serialize-test.cc@242
PS1, Line 242:         int len = rand() % (MAX_STRING_LEN + 1);
> I think we want to generate empty strings as random values. Why not just ma
Good point! Done.



-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 17 Nov 2018 16:07:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 3: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 19:23:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3477/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 3
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 19:23:42 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................

IMPALA-5031: Zero-length arrays are undefined behavior

This patch prevents the creation of a zero-length array. This is
illegal under paragraph 5 of §6.7.5.2 ("Array declarators") of the C99
standard, which reads:

  If the size is an expression that is not an integer constant
  expression: if it occurs in a declaration at function prototype
  scope, it is treated as if it were replaced by *; otherwise, each
  time it is evaluated it shall have a value greater than zero.

Variable-length arrays are not part of C++14, but they are a common
compiler extension and are available in both clang++ and g++. That the
semantics of missing features are the same is implied by [intro.scope]
in the C++14 standard, which reads:

  C++ is a general purpose programming language based on the C
  programming language as described in ISO/IEC 9899:1999 Programming
  languages — C (hereinafter referred to as the C standard).

Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Reviewed-on: http://gerrit.cloudera.org:8080/11811
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/row-batch-serialize-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 4
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 19:09:48 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11811 )

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1390/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 17 Nov 2018 16:46:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5031: Zero-length arrays are undefined behavior

Posted by "Jim Apple (Code Review)" <ge...@cloudera.org>.
Hello Lars Volker, Tim Armstrong, Impala Public Jenkins, Jinchul Kim, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/11811

to look at the new patch set (#2).

Change subject: IMPALA-5031: Zero-length arrays are undefined behavior
......................................................................

IMPALA-5031: Zero-length arrays are undefined behavior

This patch prevents the creation of a zero-length array. This is
illegal under paragraph 5 of §6.7.5.2 ("Array declarators") of the C99
standard, which reads:

  If the size is an expression that is not an integer constant
  expression: if it occurs in a declaration at function prototype
  scope, it is treated as if it were replaced by *; otherwise, each
  time it is evaluated it shall have a value greater than zero.

Variable-length arrays are not part of C++14, but they are a common
compiler extension and are available in both clang++ and g++. That the
semantics of missing features are the same is implied by [intro.scope]
in the C++14 standard, which reads:

  C++ is a general purpose programming language based on the C
  programming language as described in ISO/IEC 9899:1999 Programming
  languages — C (hereinafter referred to as the C standard).

Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
---
M be/src/runtime/row-batch-serialize-test.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/11/11811/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11811
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93f7d0b0506e4b6a2ff3303477d887a428431f96
Gerrit-Change-Number: 11811
Gerrit-PatchSet: 2
Gerrit-Owner: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jinchul Kim <ji...@gmail.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>