Bug 44466 - Fix gcc warning introduced in 65731
Summary: Fix gcc warning introduced in 65731
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 44468 (view as bug list)
Depends on:
Blocks: 43191
  Show dependency treegraph
 
Reported: 2010-08-23 16:29 PDT by Csaba Osztrogonác
Modified: 2010-08-24 04:02 PDT (History)
2 users (show)

See Also:


Attachments
Fix both warnings. (1.75 KB, patch)
2010-08-24 01:35 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff
Fix both warnings. (1.46 KB, patch)
2010-08-24 01:36 PDT, Pavel Podivilov
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Csaba Osztrogonác 2010-08-23 16:29:44 PDT
This warning introduced in http://trac.webkit.org/changeset/65731

gcc warning:
../../../WebCore/inspector/InspectorDOMAgent.cpp:1064: warning: control reaches end of non-void function

1055	bool InspectorDOMAgent::pauseOnBreakpoint() 
1056	{ 
1057	#if ENABLE(JAVASCRIPT_DEBUGGER) 
1058	    s_domAgentOnBreakpoint = this; 
1059	    ScriptDebugServer::shared().breakProgram(); 
1060	    bool deleted = !s_domAgentOnBreakpoint; 
1061	    s_domAgentOnBreakpoint = 0; 
1062	    return !deleted; 
1063	#endif 
1064	}

InspectorDOMAgent::pauseOnBreakpoint() should return a 
default value when ENABLE(JAVASCRIPT_DEBUGGER) is false.
Could you fix it, please?
Comment 1 Pavel Podivilov 2010-08-24 01:35:27 PDT
Created attachment 65232 [details]
Fix both warnings.
Comment 2 Pavel Podivilov 2010-08-24 01:36:17 PDT
Created attachment 65233 [details]
Fix both warnings.
Comment 3 Csaba Osztrogonác 2010-08-24 01:55:57 PDT
-        mask = mask | (mask >> domBreakpointDerivedTypeShift) & ((1 << domBreakpointDerivedTypeShift) - 1);
+        mask = (mask | (mask >> domBreakpointDerivedTypeShift)) & ((1 << domBreakpointDerivedTypeShift) - 1);

I don't understand what should this code do, but adding 
parantheses like this, will change the behaviour of the code.
Which one is wrong? The original or the modified?

a | b & c is equal to a | (b & c) , because & has higher precedence than |
Comment 4 Yury Semikhatsky 2010-08-24 01:59:10 PDT
(In reply to comment #3)
> -        mask = mask | (mask >> domBreakpointDerivedTypeShift) & ((1 << domBreakpointDerivedTypeShift) - 1);
> +        mask = (mask | (mask >> domBreakpointDerivedTypeShift)) & ((1 << domBreakpointDerivedTypeShift) - 1);
> 
> I don't understand what should this code do, but adding 
> parantheses like this, will change the behaviour of the code.
> Which one is wrong? The original or the modified?
> 
> a | b & c is equal to a | (b & c) , because & has higher precedence than |

The latter version is correct, original one had a bug since we want new mask to fit into domBreakpointDerivedTypeShift bits. Thanks for pointing this out.
Comment 5 Csaba Osztrogonác 2010-08-24 02:01:43 PDT
*** Bug 44468 has been marked as a duplicate of this bug. ***
Comment 6 Csaba Osztrogonác 2010-08-24 04:02:16 PDT
Landed in http://trac.webkit.org/changeset/65887