Bug 44466

Summary:

Fix gcc warning introduced in 65731

Product:

WebKit

Reporter:

Csaba Osztrogonác <ossy>

Component:

New Bugs

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

podivilov, yurys

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

43191

Attachments:

Description	Flags
Fix both warnings.	none
Fix both warnings.	none

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