[EFL] [WK2] Don't call eina_iterator_free() if iterator is NULL

[Sudarsana Nagineni (babu)]

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

[Sudarsana Nagineni (babu)]

Created attachment 149749 [details]
Patch

[Raphael Kubo da Costa (:rakuco)]

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.

[Sudarsana Nagineni (babu)]

(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.

[Raphael Kubo da Costa (:rakuco)]

I'd say it's a bug in Eina. I'll mail enlightenment-devel about this.

[Raphael Kubo da Costa (:rakuco)]

(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

[Sudarsana Nagineni (babu)]

(In reply to comment #5)
> http://article.gmane.org/gmane.comp.window-managers.enlightenment.devel/42486

Thanks for discussing this on enlightenment-devel list.

[Chris Dumez]

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" :)

[Chris Dumez]

Comment on attachment 149749 [details]
Patch

LGTM. Could we please land this ASAP? The output on stderr looks really bad.

[Raphael Kubo da Costa (:rakuco)]

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...

[Raphael Kubo da Costa (:rakuco)]

(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/[e-mail]/msg42944.html

[Kenneth Rohde Christiansen]

(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/[e-mail]/msg42944.html

Why not fix it in webkit for now adding a comment referring to this bug report?

[Raphael Kubo da Costa (:rakuco)]

That's also possible, provided we don't forget to work on this later (it's likely we will).

[Sudarsana Nagineni (babu)]

Created attachment 150561 [details]
patch

Added a comment. We can remove this once the magic check on _free removed in Eina.

[WebKit Review Bot]

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.

[Sudarsana Nagineni (babu)]

Created attachment 150562 [details]
Patch

[WebKit Review Bot]

Comment on attachment 150562 [details]
Patch

Clearing flags on attachment: 150562

Committed r121752: <http://trac.webkit.org/changeset/121752>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.