Page MenuHomePhabricator

edje_entry: avoid strstr undefined behaviour
ClosedPublic

Authored by ali.alzyod on Dec 14 2019, 6:42 AM.

Details

Summary

elm_entry: avoid strstr undefined behaviour

strstr behaviour is undefined when passing null to it, we will check if null is passed, then skip.
elm_entry had issue, where crash happened when click on link for example.

elementry_test -> entry -> click on link (crash will happened)

T8535

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 14961
Build 10299: arc lint + arc unit
ali.alzyod created this revision.Dec 14 2019, 6:42 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

ali.alzyod requested review of this revision.Dec 14 2019, 6:42 AM
ali.alzyod retitled this revision from edje_entry: real part protection check to elm_entry: avoid strstr undefined behaviour.

I don't like this if-if structure. Yes I see it's been there before, but this could be a good moment to solve this as well

if (style_str && tag)
           {
              if (strstr(style_str, tag)) return EINA_TRUE;
           }
ali.alzyod updated this revision to Diff 27556.Dec 15 2019, 8:22 PM
  • change if statment structure
ali.alzyod edited the summary of this revision. (Show Details)
ProhtMeyhet added inline comments.Dec 17 2019, 7:03 PM
src/lib/edje/edje_entry.c
808

couldn't this !tag check be done at the beginning of the function? It would save calling other functions.

823

one new line too much

825

phab syntax highlight indicates a different indentation between the two if's. like a tab vs. space. Is that the case?

ali.alzyod marked 2 inline comments as done.Dec 17 2019, 8:43 PM
ali.alzyod added inline comments.
src/lib/edje/edje_entry.c
825

I do not understand what do you mean

ProhtMeyhet added inline comments.Dec 17 2019, 8:47 PM
src/lib/edje/edje_entry.c
825

then it is probably ok. :-)

ProhtMeyhet added inline comments.Dec 17 2019, 8:49 PM
src/lib/edje/edje_entry.c
802

shouldn't it be return EINA_FALSE?

ali.alzyod marked an inline comment as done.Dec 17 2019, 9:14 PM

Thank you, this looks much better than before.

bu5hm4n accepted this revision.Dec 18 2019, 12:18 AM
This revision is now accepted and ready to land.Dec 18 2019, 12:18 AM
bu5hm4n requested changes to this revision.Dec 18 2019, 12:20 AM
bu5hm4n added inline comments.
src/lib/edje/edje_entry.c
824

does not compile

This revision now requires changes to proceed.Dec 18 2019, 12:20 AM

fix compilation error

ali.alzyod marked an inline comment as done.Dec 19 2019, 10:13 PM
zmike accepted this revision.Dec 23 2019, 6:39 AM

Seems good, though the commit log should say edje_entry instead of elm_entry since this isn't an elm change.

Deferring for @bu5hm4n to reread since he blocked it last.

zmike added a comment.Dec 23 2019, 6:40 AM

Would be good to get a unit test for this to ensure it doesn't happen again since I imagine this is a regular case

bu5hm4n accepted this revision.Dec 23 2019, 8:26 AM
bu5hm4n retitled this revision from elm_entry: avoid strstr undefined behaviour to edje_entry: avoid strstr undefined behaviour.

@ali.alzyod Can you create a unit test ?

This revision is now accepted and ready to land.Dec 23 2019, 8:26 AM
Closed by commit rEFLed0572a33a28: edje_entry: avoid strstr undefined behaviour (authored by ali.alzyod, committed by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyDec 23 2019, 8:43 AM
This revision was automatically updated to reflect the committed changes.