Bug 192086

Summary:

[GTK][WPE] Fix BubblewrapLauncher clang warnings

Product:

WebKit

Reporter:

Tomas Popela <tpopela>

Component:

WebKitGTK

Assignee:

Tomas Popela <tpopela>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bugs-noreply, commit-queue, mcatanzaro

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Tomas Popela

Reported 2018-11-28 10:18:52 PST

../../Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:474:37: warning: variable 'i' is uninitialized when used here [-Wuninitialized] for (size_t i; splitPaths.get()[i]; ++i) ^ ../../Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:474:18: note: initialize the variable 'i' to silence this warning for (size_t i; splitPaths.get()[i]; ++i) ^ = 0 ../../Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:574:34: warning: missing field 'datum_b' initializer [-Wmissing-field-initializers] struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (int)TIOCSTI); ^ /usr/include/seccomp.h:211:23: note: expanded from macro 'SCMP_A1' #define SCMP_A1(...) SCMP_CMP(1, __VA_ARGS__) ^ /usr/include/seccomp.h:201:58: note: expanded from macro 'SCMP_CMP' #define SCMP_CMP(...) ((struct scmp_arg_cmp){__VA_ARGS__}) ^

Attachments
Patch (1.96 KB, patch) 2018-11-28 10:20 PST, Tomas Popela	no flags	Details Formatted Diff Diff
Patch for landing (1.98 KB, patch) 2018-11-29 01:02 PST, Tomas Popela	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Tomas Popela

Comment 1 2018-11-28 10:20:56 PST

Created attachment 355886 [details] Patch

Michael Catanzaro

Comment 2 2018-11-28 10:42:45 PST

Comment on attachment 355886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355886&action=review > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:474 > - for (size_t i; splitPaths.get()[i]; ++i) > + for (size_t i = 0; splitPaths.get()[i]; ++i) Ow! > Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:574 > - struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (int)TIOCSTI); > + struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (scmp_datum_t)TIOCSTI, 0); I spent too long investigating why you have the trailing 0. Would be more clear if we add an unnecessary cast: struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (scmp_datum_t)TIOCSTI, (scmp_datum_t)0); But now I notice that we are impermissibly using C-style casts here. How about: struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, static_cast<scmp_datum_t>(TIOCSTI), static_cast<scmp_datum_t>(0)); Er, maybe too verbose. It's probably fine with just the first cast. But it should be a static_cast.

Tomas Popela

Comment 3 2018-11-28 12:28:42 PST

Comment on attachment 355886 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=355886&action=review >> Source/WebKit/UIProcess/Launcher/glib/BubblewrapLauncher.cpp:574 >> + struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (scmp_datum_t)TIOCSTI, 0); > > I spent too long investigating why you have the trailing 0. Would be more clear if we add an unnecessary cast: > > struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, (scmp_datum_t)TIOCSTI, (scmp_datum_t)0); > > But now I notice that we are impermissibly using C-style casts here. How about: > > struct scmp_arg_cmp ttyArg = SCMP_A1(SCMP_CMP_EQ, static_cast<scmp_datum_t>(TIOCSTI), static_cast<scmp_datum_t>(0)); > > Er, maybe too verbose. It's probably fine with just the first cast. But it should be a static_cast. True, that sounds a lot better (I don't know, why I didn't do that)! :)

Tomas Popela

Comment 4 2018-11-29 01:02:45 PST

Created attachment 355985 [details] Patch for landing

WebKit Commit Bot

Comment 5 2018-11-29 01:53:06 PST

The commit-queue encountered the following flaky tests while processing attachment 355985 [details]: workers/bomb.html bug 171985 (author: fpizlo@apple.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 6 2018-11-29 01:53:25 PST

The commit-queue encountered the following flaky tests while processing attachment 355985 [details]: imported/w3c/web-platform-tests/css/css-shapes/shape-outside/values/shape-outside-inset-001.html bug 192139 (author: rego@igalia.com) imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form.html bug 192140 (author: mjs@apple.com) webgl/1.0.2/conformance/more/conformance/quickCheckAPI-A.html bug 192141 (author: roger_fong@apple.com) The commit-queue is continuing to process your patch.

WebKit Commit Bot

Comment 7 2018-11-29 02:19:25 PST

WebKit Commit Bot

Comment 8 2018-11-29 02:20:03 PST

Comment on attachment 355985 [details] Patch for landing Clearing flags on attachment: 355985 Committed r238663: <https://trac.webkit.org/changeset/238663>

WebKit Commit Bot

Comment 9 2018-11-29 02:20:05 PST

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.