[webkit-reviews] review granted: [Bug 86961] [EFL] Check if ecore_x is initialised before calling ecore_x_bell to avoid crash : [Attachment 162007] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Alexander Shalamov
<alexander.shalamov at gmail.com>'s request for review:
Bug 86961: [EFL] Check if ecore_x is initialised before calling ecore_x_bell to
avoid crash
https://bugs.webkit.org/show_bug.cgi?id=86961

Attachment 162007: Patch
https://bugs.webkit.org/attachment.cgi?id=162007&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=162007&action=review


>>>>>> Source/WebCore/platform/efl/SoundEfl.cpp:42
>>>>>> +    if (ecore_x_init(0)) {
>>>>> 
>>>>> This potentially initializes ecoreX multiple times, right? How about a
function level static variable here to remember whether initialization
completed?
>>>> 
>>>> No it does not. It increments a ref count.
>>> 
>>> ecore_x_* has static variable for ref counting, all init / shutdown calls
would increment / decrement ref count value.
>> 
>> why not use the new WITH_ECORE_X ?
> 
> The macro was fetching screen to get UI related properties.
> Screen or display is not needed to call a bell (ecore_x_bell).

I guess it should be renamed then to ECORE_X_CALL_WITH_DISPLAY or so then to
make that more clear