Summary: | [Seccomp] Should be easier to debug blocked syscalls | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Catanzaro <mcatanzaro> | ||||||||||||
Component: | WebKit2 | Assignee: | Michael Catanzaro <mcatanzaro> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Enhancement | CC: | commit-queue, mcatanzaro, zan | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | PC | ||||||||||||||
OS: | Linux | ||||||||||||||
Bug Depends on: | 150172 | ||||||||||||||
Bug Blocks: | 140072 | ||||||||||||||
Attachments: |
|
Description
Michael Catanzaro
2015-03-23 13:45:24 PDT
Created attachment 249264 [details]
[Linux] Seccomp Filters: Make it easier to debug blocked syscalls
Attachment 249264 [details] did not pass style-queue:
ERROR: Source/WebKit2/Shared/linux/SeccompFilters/SeccompBroker.cpp:308: Multi line control clauses should use braces. [whitespace/braces] [4]
ERROR: Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:103: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
ERROR: Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp:78: msg_needed is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp:81: write_uint is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4]
ERROR: Source/WebKit2/Shared/linux/SeccompFilters/Syscall.cpp:86: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5]
Total errors found: 5 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 249265 [details]
[Linux] Seccomp Filters: Add helper functions for common directories
Comment on attachment 249265 [details] [Linux] Seccomp Filters: Add helper functions for common directories $ webkit-patch post-commits 7f1dedab84e00f593d6618 --no-review bugs.webkit.org login: mcatanzaro@igalia.com Logging in as mcatanzaro@igalia.com... Fetching: https://bugs.webkit.org/show_bug.cgi?id=142980&ctype=xml&excludefield=attachmentdata Obsoleting 1 old patch on bug 142980 Obsoleting attachment: 249264 Adding patch "[Linux] Seccomp Filters: Add helper functions for common directories" to https://bugs.webkit.org/show_bug.cgi?id=142980 Looks like a bug in webkit-patch, since the URL in the patch is clearly https://bugs.webkit.org/show_bug.cgi?id=142982 Created attachment 249274 [details]
[Linux] Seccomp Filters: Make it easier to debug blocked syscalls
Created attachment 257067 [details]
Patch
Comment on attachment 257067 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257067&action=review > Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:277 > + CRASH(); ASSERT_NOT_REACHED() maybe, with a default return value? The compiler might also be smart and verbose enough to throw a warning/error when all the enum values are not accounted for in a switch statement with no default label. (In reply to comment #7) > Comment on attachment 257067 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257067&action=review > > > Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:277 > > + CRASH(); > > ASSERT_NOT_REACHED() maybe, with a default return value? OK > The compiler might > also be smart and verbose enough to throw a warning/error when all the enum > values are not accounted for in a switch statement with no default label. It will do so if we use -Wswitch (we do; it's implied by -Wall), and it will do so _even with a default label_ if we use -Wswitch-enum (we don't, thank goodness). Created attachment 257114 [details]
Patch
Comment on attachment 257114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257114&action=review > Source/WebKit2/Shared/linux/SeccompFilters/SyscallPolicy.cpp:279 > + return ""; I'd prefer 'unknown' or similar just so that the message doesn't get confusing and discarded because of an empty permission name. Thanks, I'll go with "unknown action". Committed r187030: <http://trac.webkit.org/changeset/187030> |