Page MenuHomePhabricator

efl/timer: correctly handle timer freeze during construction
ClosedPublic

Authored by zmike on Aug 8 2018, 11:33 AM.

Details

Summary

it is not correct to throw an error when methods are called during
construction

Diff Detail

Repository
rEFL core/efl
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
zmike created this revision.Aug 8 2018, 11:33 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/

zmike requested review of this revision.Aug 8 2018, 11:33 AM
Hermet requested changes to this revision.Aug 16 2018, 10:26 PM
Hermet added a subscriber: Hermet.

Could u please check a comment?

src/lib/ecore/ecore_timer.c
331–335

Wondering this case timer->at has what value....?
Looks timer->pending could be invalid. must be take care of it.

This revision now requires changes to proceed.Aug 16 2018, 10:26 PM
zmike planned changes to this revision.Aug 17 2018, 6:14 AM
zmike added inline comments.
src/lib/ecore/ecore_timer.c
331–335

Good catch, this could have triggered a subtle bug.

zmike updated this revision to Diff 16384.Aug 17 2018, 6:28 AM

be more careful when applying freeze

Hermet requested changes to this revision.Aug 19 2018, 10:23 PM

one more question.

src/lib/ecore/ecore_timer.c
331–335

hmm, timer->at won't be not initialized yet here.
Guess It must be this .. if (timer uninitialized) timer->at = timer->pending = 0;

This revision now requires changes to proceed.Aug 19 2018, 10:23 PM
zmike requested review of this revision.Aug 20 2018, 4:07 AM
zmike added inline comments.
src/lib/ecore/ecore_timer.c
331–335

Right, if the timer is still being created then at and pending are both 0.0 as a result of calloc.

Hermet added inline comments.Aug 20 2018, 9:51 PM
src/lib/ecore/ecore_timer.c
331–335

if at == 0, timer->pending must be invalid. that's a problem. (see timer->at{0} - now)
So I'd rather look this.
if (!timer->initialized)

timer->pending = 0;

else

timer->pending = timer->at - now;
zmike updated this revision to Diff 16456.Aug 21 2018, 7:25 AM

add explicit initialized check

Hermet accepted this revision.Aug 26 2018, 8:08 PM
This revision is now accepted and ready to land.Aug 26 2018, 8:08 PM
This revision was automatically updated to reflect the committed changes.