Bug 64932

Summary: [EFL] Add NULL checks to ewk_window_features_new_from_core and ewk_view_window_create.
Product: WebKit Reporter: Grzegorz Czajkowski <g.czajkowski>
Component: WebKit EFLAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, lucas.de.marchi, rakuco, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Linux   
Attachments:
Description Flags
proposed patch
none
updated patch
eric: review+, webkit.review.bot: commit-queue-
proposed patch none

Description Grzegorz Czajkowski 2011-07-21 03:43:45 PDT
It prevents the crash while allocating a memory for the new window.
Comment 1 Grzegorz Czajkowski 2011-07-21 03:45:13 PDT
Created attachment 101569 [details]
proposed patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-07-21 06:13:09 PDT
Did this actually happen to you when you were doing some tests, or is this a hypothetical case?

> Source/WebKit/efl/ChangeLog:3
> +        [EFL] Added NULL cheking in ewk_window_features_new_from_core and ewk_view_window_create.

checking -> checking

In general, these descriptions are in the present tense, so it might be better phrased as "Add NULL checks to ewk_window_features_new_from_core and ewk_view_window_create".

> Source/WebKit/efl/ChangeLog:6
> +        It prevents the crash while allocating a memory for the new window.

the -> a
a memory -> memory
Comment 3 Grzegorz Czajkowski 2011-07-21 06:25:42 PDT
(In reply to comment #2)
> Did this actually happen to you when you were doing some tests, or is this a hypothetical case?
> 
This is a hypothetical case:) Generally we check malloc (and friend) returned value in WebKit-EFL. Just this is one them :)
Comment 4 Grzegorz Czajkowski 2011-07-21 07:07:01 PDT
Created attachment 101584 [details]
updated patch

Fixed patch according to Raphael's suggestions.
Comment 5 Raphael Kubo da Costa (:rakuco) 2011-07-21 08:00:11 PDT
r+ from my side.
Comment 6 Grzegorz Czajkowski 2011-07-21 08:01:44 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Did this actually happen to you when you were doing some tests, or is this a hypothetical case?
> > 
> This is a hypothetical case:) Generally we check malloc (and friend) returned value in WebKit-EFL. Just this is one them :)

One more thing. This is one of important defects in WebKit-EFL which was found by static analysis tools. These tools sometimes provide not reliable data. We should appraise whether this defect is really bug for us or not :) I think we should fix it in this case.
Comment 7 Gyuyoung Kim 2011-08-08 01:47:40 PDT
Comment on attachment 101584 [details]
updated patch

LGTM. I think this patch needs to be landed.
Comment 8 Eric Seidel (no email) 2011-09-12 15:48:10 PDT
Comment on attachment 101584 [details]
updated patch

rs=me.
Comment 9 WebKit Review Bot 2011-09-12 20:13:59 PDT
Comment on attachment 101584 [details]
updated patch

Rejecting attachment 101584 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
ebKit/efl/ewk/ewk_view.cpp
Hunk #1 succeeded at 2864 (offset -809 lines).
Hunk #2 FAILED at 3685.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_view.cpp.rej
patching file Source/WebKit/efl/ewk/ewk_window_features.cpp
Hunk #1 FAILED at 150.
1 out of 1 hunk FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_window_features.cpp.rej

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 1

Full output: http://queues.webkit.org/results/9648315
Comment 10 Grzegorz Czajkowski 2011-09-14 05:44:52 PDT
Created attachment 107321 [details]
proposed patch

Patch has been re-based.
Comment 11 Gyuyoung Kim 2011-09-14 05:50:40 PDT
Comment on attachment 107321 [details]
proposed patch

LGTM.
Comment 12 WebKit Review Bot 2011-09-14 06:51:35 PDT
Comment on attachment 107321 [details]
proposed patch

Clearing flags on attachment: 107321

Committed r95088: <http://trac.webkit.org/changeset/95088>
Comment 13 WebKit Review Bot 2011-09-14 06:51:41 PDT
All reviewed patches have been landed.  Closing bug.