Bug 90076

Summary: [EFL] [WK2] Don't call eina_iterator_free() if iterator is NULL
Product: WebKit Reporter: Sudarsana Nagineni (babu) <naginenis>
Component: WebKit EFLAssignee: Sudarsana Nagineni (babu) <naginenis>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, gyuyoung.kim, kenneth, lucas.de.marchi, rakuco, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
patch
none
Patch none

Description Sudarsana Nagineni (babu) 2012-06-27 07:55:14 PDT
I'm seeing the following console log messages when Minibrowser opened.

CRI<32386>: eina_iterator.c:98 eina_iterator_free() *** Eina Magic Check Failed !!!
    Input handle pointer is NULL !
*** NAUGHTY PROGRAMMER!!!
*** SPANK SPANK SPANK!!!
*** Now go fix your code. Tut tut tut!


ERR<32386>: eina_iterator.c:99 eina_iterator_free() safety check failed: iterator == NULL
Comment 1 Sudarsana Nagineni (babu) 2012-06-27 08:13:32 PDT
Created attachment 149749 [details]
Patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2012-06-27 12:10:02 PDT
Comment on attachment 149749 [details]
Patch

Is this really needed? eina_iterator_free() should be a NOP if the iterator passed to it is NULL.
Comment 3 Sudarsana Nagineni (babu) 2012-06-27 12:53:38 PDT
(In reply to comment #2)
> (From update of attachment 149749 [details])
> Is this really needed? eina_iterator_free() should be a NOP if the iterator passed to it is NULL.

(In reply to comment #2)
> (From update of attachment 149749 [details])
> Is this really needed? eina_iterator_free() should be a NOP if the iterator passed to it is NULL.

We need it to get rid of these critical error messages on browser startup.
Comment 4 Raphael Kubo da Costa (:rakuco) 2012-06-27 15:14:41 PDT
I'd say it's a bug in Eina. I'll mail enlightenment-devel about this.
Comment 5 Raphael Kubo da Costa (:rakuco) 2012-06-27 15:26:22 PDT
(In reply to comment #4)
> I'd say it's a bug in Eina. I'll mail enlightenment-devel about this.

http://article.gmane.org/gmane.comp.window-managers.enlightenment.devel/42486
Comment 6 Sudarsana Nagineni (babu) 2012-06-28 00:58:22 PDT
(In reply to comment #5)
> http://article.gmane.org/gmane.comp.window-managers.enlightenment.devel/42486

Thanks for discussing this on enlightenment-devel list.
Comment 7 Chris Dumez 2012-06-28 01:22:35 PDT
And the doc for eina_iterator_free() says it frees the iterator "if the iterator is not NULL". So the doc seems to hint that passing NULL is possible but then if you do it you get "SPANKED" :)
Comment 8 Chris Dumez 2012-07-02 04:08:50 PDT
Comment on attachment 149749 [details]
Patch

LGTM. Could we please land this ASAP? The output on stderr looks really bad.
Comment 9 Raphael Kubo da Costa (:rakuco) 2012-07-02 07:00:47 PDT
I still think it makes more sense to fix this one in Eina, I just haven't had time to follow up with a patch in that thread...
Comment 10 Raphael Kubo da Costa (:rakuco) 2012-07-02 08:42:28 PDT
(In reply to comment #9)
> I still think it makes more sense to fix this one in Eina, I just haven't had time to follow up with a patch in that thread...

I have now sent http://www.mail-archive.com/enlightenment-devel@lists.sourceforge.net/msg42944.html
Comment 11 Kenneth Rohde Christiansen 2012-07-02 18:46:21 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > I still think it makes more sense to fix this one in Eina, I just haven't had time to follow up with a patch in that thread...
> 
> I have now sent http://www.mail-archive.com/enlightenment-devel@lists.sourceforge.net/msg42944.html

Why not fix it in webkit for now adding a comment referring to this bug report?
Comment 12 Raphael Kubo da Costa (:rakuco) 2012-07-02 21:01:17 PDT
That's also possible, provided we don't forget to work on this later (it's likely we will).
Comment 13 Sudarsana Nagineni (babu) 2012-07-03 02:04:36 PDT
Created attachment 150561 [details]
patch

Added a comment. We can remove this once the magic check on _free removed in Eina.
Comment 14 WebKit Review Bot 2012-07-03 02:06:34 PDT
Attachment 150561 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/efl/FileSystemEfl.cpp:94:  Use 0 or null instead of NULL (even in *comments*).  [readability/null] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Sudarsana Nagineni (babu) 2012-07-03 02:11:08 PDT
Created attachment 150562 [details]
Patch
Comment 16 WebKit Review Bot 2012-07-03 03:04:27 PDT
Comment on attachment 150562 [details]
Patch

Clearing flags on attachment: 150562

Committed r121752: <http://trac.webkit.org/changeset/121752>
Comment 17 WebKit Review Bot 2012-07-03 03:04:32 PDT
All reviewed patches have been landed.  Closing bug.