Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2512073003:
Aarch64 buildfix. (Closed)
DescriptionAarch64 buildfix.
Cast ints to the proper enum type to make clang happy.
BUG=666780
Committed: https://crrev.com/033fbea1cde6401bacabe4b721f21ba41215ee23
Cr-Commit-Position: refs/heads/master@{#434145}
Patch Set 1 #
Total comments: 3
Patch Set 2 : Aarch64 buildfix. #Patch Set 3 : Aarch64 buildfix. #
Total comments: 4
Patch Set 4 : Aarch64 buildfix. #Messages
Total messages: 27 (9 generated)
Description was changed from ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 ========== to ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 ==========
ossy.szeged@gmail.com changed reviewers: + jorgelo@chromium.org
Have you signed the Chromium CLA? https://dev.chromium.org/developers/contributing-code/external-contributor-ch... https://codereview.chromium.org/2512073003/diff/1/sandbox/linux/integration_t... File sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc (right): https://codereview.chromium.org/2512073003/diff/1/sandbox/linux/integration_t... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1998: BPF_ASSERT_NE(-1, ptrace(static_cast<enum __ptrace_request>(PTRACE_GETREGS), pid, NULL, ®s)); This line is longer than 80 cols. https://codereview.chromium.org/2512073003/diff/1/sandbox/linux/integration_t... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:2006: BPF_ASSERT_NE(-1, ptrace(static_cast<enum __ptrace_request>(PTRACE_SETREGS), pid, NULL, ®s)); And this one. https://codereview.chromium.org/2512073003/diff/1/sandbox/linux/integration_t... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:2014: BPF_ASSERT_NE(-1, ptrace(static_cast<enum __ptrace_request>(PTRACE_SETREGS), pid, NULL, ®s)); And this one too.
On 2016/11/18 19:54:01, Jorge Lucangeli Obes wrote: > Have you signed the Chromium CLA? > https://dev.chromium.org/developers/contributing-code/external-contributor-ch... Yes, and added myself to AUTHORS file, but that CL hasn't been landed yet - https://codereview.chromium.org/2511363002/ Additionally I already fixed the style issues.
Looks like https://codereview.chromium.org/2511363002/ has landed so this LGTM.
The CQ bit was checked by pgal.u-szeged@partner.samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
On 2016/11/22 15:27:03, commit-bot: I haz the power wrote: > Try jobs failed on following builders: > android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, > https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) > android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, > https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) ../../sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1999:26: error: use of enum '__ptrace_request' without previous declaration static_cast<enum __ptrace_request>(PTRACE_GETREGS), pid, NULL, ®s)); ^ Strange. Isn't there enum __ptrace_request declaration on Android's sys/ptrace.h ?
On 2016/11/22 15:40:02, Ossy wrote: > On 2016/11/22 15:27:03, commit-bot: I haz the power wrote: > > Try jobs failed on following builders: > > android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, > > > https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...) > > android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, > > > https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...) > > ../../sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1999:26: > error: use of enum '__ptrace_request' without previous declaration > static_cast<enum __ptrace_request>(PTRACE_GETREGS), pid, NULL, ®s)); > ^ > > Strange. Isn't there enum __ptrace_request declaration on Android's sys/ptrace.h > ? Looks like enum __ptrace_request is a GLIBC extension. It's not in POSIX: https://github.com/openembedded/openembedded-core/blob/master/meta/recipes-ex...
My idea is to add casting to the define lines with !ANDROID guard. Or should we use the GLIBC guard instead? Is it available here?
On 2016/11/22 15:57:57, Ossy wrote: > My idea is to add casting to the define lines with !ANDROID guard. > Or should we use the GLIBC guard instead? Is it available here? I thikn you should use a GLIBC guard -- that's more semantically correct.
I updated the patch with GLIBC guard and try bots are happy now.
https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... File sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc (left): https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1881: #endif We have too many endifs. Let's comment what they do. #endif // defined(__GLIBC__) #endif // !defined(PTRACE_GETREGS) #endif // defined(__aarch64) https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1887: #endif Same: #endif // defined(__GLIBC__) #endif // !defined(PTRACE_SETREGS) #endif // defined(__aarch64) https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... File sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc (right): https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1880: #ifdef __GLIBC__ Please use #if defined(...) here. https://codereview.chromium.org/2512073003/diff/40001/sandbox/linux/integrati... sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc:1890: #ifdef __GLIBC__ Same.
Patchset #4 (id:60001) has been deleted
fixed comments and used defined(__GLIBC__) instead of ifdef
Thank you! lgtm
The CQ bit was checked by pgal.u-szeged@partner.samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1479900067757480,
"parent_rev": "f18c542f88bd0bbed58a70e24ba383cfed47f4ad", "commit_rev":
"540e26d7efca995bb470ced562edfa98bfb9d659"}
Message was sent while issue was closed.
Description was changed from ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 ========== to ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 ========== to ========== Aarch64 buildfix. Cast ints to the proper enum type to make clang happy. BUG=666780 Committed: https://crrev.com/033fbea1cde6401bacabe4b721f21ba41215ee23 Cr-Commit-Position: refs/heads/master@{#434145} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/033fbea1cde6401bacabe4b721f21ba41215ee23 Cr-Commit-Position: refs/heads/master@{#434145} |
