Bug 98174 – Unix plugin process: xErrorString does not need to be defined in release builds

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adrian Perez]

Unix plugin process: xErrorString does not need to be defined in release builds

[Adrian Perez]

Created attachment 166707 [details]
Patch

[Adrian Perez]

The patch fixes this warning in release mode (well, in general whenever LOG_DISABLE is defined):

../../Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:48:20: warning: ‘WebKit::xErrorString’ defined but not used [-Wunused-variable]

[Darin Adler]

Comment on attachment 166707 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166707&action=review

> Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:54
> +#ifndef LOG_DISABLED
>  static const char* xErrorString = "The program '%s' received an X Window System error.\n"
>      "This probably reflects a bug in a browser plugin.\n"
>      "The error was '%s'.\n"
>      "  (Details: serial %ld error_code %d request_code %d minor_code %d)\n";
> +#endif /* !LOG_DISABLED */

Could also make this a bit more const. Either const char[] or const char* const.

[Adrian Perez]

(In reply to comment #3)
> (From update of attachment 166707 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=166707&action=review
> 
> > Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:54
> > +#ifndef LOG_DISABLED
> >  static const char* xErrorString = "The program '%s' received an X Window System error.\n"
> >      "This probably reflects a bug in a browser plugin.\n"
> >      "The error was '%s'.\n"
> >      "  (Details: serial %ld error_code %d request_code %d minor_code %d)\n";
> > +#endif /* !LOG_DISABLED */
> 
> Could also make this a bit more const. Either const char[] or const char* const.

Sure, thanks. Will make it “const char[]” and re-upload the patch. Thanks!

[Adrian Perez]

Created attachment 166712 [details]
Patch

[WebKit Review Bot]

Comment on attachment 166712 [details]
Patch

Clearing flags on attachment: 166712

Committed r130225: <http://trac.webkit.org/changeset/130225>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

This patch broke debug build on EFL port it seems:
/home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp: In function ‘int WebKit::webkitXError(Display*, XErrorEvent*)’:
/home/buildslave-1/webkit-buildslave/efl-linux-64-debug/build/Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:63:7: error: ‘xErrorString’ was not declared in this scope
make[2]: *** [Source/WebKit2/CMakeFiles/ewebkit2.dir/PluginProcess/unix/PluginProcessMainUnix.cpp.o] Error 1

[Chris Dumez]

Comment on attachment 166712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166712&action=review

> Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:50
> +static const char xErrorString[] = "The program '%s' received an X Window System error.\n"

xErrorString is used in webkitXError() which is not protected by #ifndef LOG_DISABLED

[Chris Dumez]

Comment on attachment 166712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=166712&action=review

> Source/WebKit2/PluginProcess/unix/PluginProcessMainUnix.cpp:49
> +#ifndef LOG_DISABLED

This should be:
#if !LOG_DISABLED

I'm fixing it in Bug 98232.