Page MenuHomePhabricator

efl_ui_calendar: Check for valid spin timer before setting interval
AbandonedPublic

Authored by Hermet on Jul 16 2018, 6:27 PM.

Details

Summary

The spin_month timer could possibly fail to be created so we should be
checking that it is valid before trying to set an interval on it.

ref T7076

@fix

elm_calendar: Check for valid spin timer before setting interval

The spin_month timer could possibly fail to be created so we should
be checking that it is valid before trying to set an interval on it.

ref T7076

@fix

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 6904
Build 6849: arc lint + arc unit
devilhorns created this revision.Jul 16 2018, 6:27 PM
devilhorns requested review of this revision.Jul 16 2018, 6:27 PM
ManMower added inline comments.Jul 16 2018, 6:33 PM
src/lib/elementary/efl_ui_calendar.c
667

Is it legitimate to get here with a NULL sd->spin_month or should this use an EINA_SAFETY macro or print an ERR of some kind for this case?

Whatever you do, remember to do it to all other spinners, since they are all assuming that sd->spin_* will be available. ALSO, I would like to know why this particular spinner is NULL.

Hermet added a reviewer: CHAN.Jul 16 2018, 7:55 PM
Hermet added a subscriber: Hermet.

@CHAN please review code.

Whatever you do, remember to do it to all other spinners, since they are all assuming that sd->spin_* will be available. ALSO, I would like to know why this particular spinner is `NULL

src/lib/elementary/efl_ui_calendar.c
667

We could trap this with an EINA_SAFETY call, sure. The real question here is Why does this calendar widget use a timer to change the month/year/etc after button response ?? The day, month, year, etc could easily be changed in response to a button press With Out the need for a timer at all....

This set of patches Does fix the Ticket because NULL is no more passed to eo function(s).

The issue I have with this widget is ... WHY is a timer needed at all here ?? I would guess that this is done because of different calendar styles/themes to add a delay or provide a method of constant repeat when button is held down.

Whatever the case is, I've no intention or desire to rewrite this widget to fix timer usage, so patch can be accepted to fix NULL issue, or patch should be abandoned.

CHAN added a comment.Jul 17 2018, 11:07 PM

Thank you @devilhorns

This is nice catch. i didn't care this much before.

i fixed this D6626

If you have any more problems or have further questions, please make a ticket or ask me.

@devilhorns if you agree on @CHAN, I will review his patch and we can discard this one. :)

@Hermet Sure, his other patch set looks more complete and hopefully will still fix the issue of T7076

Hermet commandeered this revision.Jul 22 2018, 11:52 PM
Hermet abandoned this revision.
Hermet added a reviewer: devilhorns.

the regarded patch has been submitted.