Bug 98174 - Unix plugin process: xErrorString does not need to be defined in release builds
Summary: Unix plugin process: xErrorString does not need to be defined in release builds
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 98232
Blocks:
  Show dependency tree / graph
 
Reported: 2012-10-02 10:20 PDT by Adrian Perez
Modified: 2012-10-02 22:58 PDT (History)
2 users (show)

See Also:

Attachments
Patch (1.78 KB, patch)
2012-10-02 10:23 PDT, Adrian Perez 		no flags Details | Formatted Diff | Diff
Patch (1.97 KB, patch)
2012-10-02 10:39 PDT, Adrian Perez 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2012-10-02 10:20:47 PDT 
Unix plugin process: xErrorString does not need to be defined in release builds
Comment 1 Adrian Perez 2012-10-02 10:23:45 PDT 
Created attachment 166707 [details]
Patch
Comment 2 Adrian Perez 2012-10-02 10:25:18 PDT 
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]
Comment 3 Darin Adler 2012-10-02 10:25:26 PDT 
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.
Comment 4 Adrian Perez 2012-10-02 10:27:15 PDT 
(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!
Comment 5 Adrian Perez 2012-10-02 10:39:57 PDT 
Created attachment 166712 [details]
Patch
Comment 6 WebKit Review Bot 2012-10-02 16:13:41 PDT 
Comment on attachment 166712 [details]
Patch

Clearing flags on attachment: 166712

Committed r130225: <http://trac.webkit.org/changeset/130225>
Comment 7 WebKit Review Bot 2012-10-02 16:13:44 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 8 Chris Dumez 2012-10-02 22:45:54 PDT 
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
Comment 9 Chris Dumez 2012-10-02 22:46:51 PDT 
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
Comment 10 Chris Dumez 2012-10-02 22:58:35 PDT 
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.