[webkit-reviews] review granted: [Bug 192086] [GTK][WPE] Fix BubblewrapLauncher clang warnings : [Attachment 355886] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at igalia.com> has granted Tomas Popela
<tpopela at redhat.com>'s request for review:
Bug 192086: [GTK][WPE] Fix BubblewrapLauncher clang warnings
https://bugs.webkit.org/show_bug.cgi?id=192086

Attachment 355886: Patch

https://bugs.webkit.org/attachment.cgi?id=355886&action=review




--- Comment #2 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 355886
  --> https://bugs.webkit.org/attachment.cgi?id=355886
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.