Bug 62630 – Remove dead code in DumpRenderTree/TestNetscapePlugIn/main.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Andras Becsi]

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.

[Csaba Osztrogonác]

(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.

[Andras Becsi]

Created attachment 97098 [details]
proposed patch

[Csaba Osztrogonác]

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.

[Andras Becsi]

(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

[Andras Becsi]

Comment on attachment 97098 [details]
proposed patch

Clearing flags.