Bug 62630 - Remove dead code in DumpRenderTree/TestNetscapePlugIn/main.cpp
Summary: Remove dead code in DumpRenderTree/TestNetscapePlugIn/main.cpp
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2011-06-14 05:12 PDT by Andras Becsi
Modified: 2011-06-14 05:33 PDT (History)
3 users (show)

See Also:

Attachments
proposed patch (1.93 KB, patch)
2011-06-14 05:16 PDT, Andras Becsi 		no flags Details | Formatted Diff | Diff
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 Andras Becsi 2011-06-14 05:12:28 PDT 
http://trac.webkit.org/changeset/88712 revealed a dead variable:

Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:135:10: error: variable 'forceCarbon' set but not used [-Werror=unused-but-set-variable]

The forceCarbon variable is initialized false in the common code-path, then checked in a section guarded by XP_MACOSX:

Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:168:    if (supportsCocoa && !forceCarbon)

and then set to true in Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:218

        else if (strcasecmp(argn[i], "forcecarbon") == 0)
            forceCarbon = true;

Since forceCarbon is always false in main.cpp:168 it is irrelevant in this function and can be removed.
Comment 1 Csaba Osztrogonác 2011-06-14 05:15:32 PDT 
(In reply to comment #0)
> http://trac.webkit.org/changeset/88712 revealed a dead variable:
> 
> Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:135:10: error: variable 'forceCarbon' set but not used [-Werror=unused-but-set-variable]
> 
> The forceCarbon variable is initialized false in the common code-path, then checked in a section guarded by XP_MACOSX:
> 
> Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:168:    if (supportsCocoa && !forceCarbon)
> 
> and then set to true in Tools/DumpRenderTree/TestNetscapePlugIn/main.cpp:218
> 
>         else if (strcasecmp(argn[i], "forcecarbon") == 0)
>             forceCarbon = true;
> 
> Since forceCarbon is always false in main.cpp:168 it is irrelevant in this function and can be removed.

I agree.

Additionally setting forceCarbon to true in main.cpp:219 is dead code,
because its value isn't used after this line. It should be removed too.
Comment 2 Andras Becsi 2011-06-14 05:16:14 PDT 
Created attachment 97098 [details]
proposed patch
Comment 3 Csaba Osztrogonác 2011-06-14 05:21:53 PDT 
Comment on attachment 97098 [details]
proposed patch

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

LGTM, r=me, but please rename the bug to "Remove dead code in DumpRenderTree/TestNetscapePlugIn/main.cpp"

> Tools/ChangeLog:5
> +        [Qt] Fix the build with gcc 4.6 after r88712.

The bug name is misleading, because it isn't Qt specific 
bug, but we need a simple dead code elimination.
Comment 4 Andras Becsi 2011-06-14 05:32:48 PDT 
(In reply to comment #3)
> (From update of attachment 97098 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97098&action=review
> 
> LGTM, r=me, but please rename the bug to "Remove dead code in DumpRenderTree/TestNetscapePlugIn/main.cpp"
> 
> > Tools/ChangeLog:5
> > +        [Qt] Fix the build with gcc 4.6 after r88712.
> 
> The bug name is misleading, because it isn't Qt specific 
> bug, but we need a simple dead code elimination.

Committed in r88791
Comment 5 Andras Becsi 2011-06-14 05:33:12 PDT 
Comment on attachment 97098 [details]
proposed patch

Clearing flags.