Page MenuHomePhabricator

efl_ui_pack: allow NULL as existing parameter in after and before
ClosedPublic

Authored by bu5hm4n on Sep 19 2019, 8:57 AM.

Details

Summary

with this commit all implementations of Efl.Pack_Linear to permit NULL
as existing parameter, this is verified with a spec test unit.

fixes T8210

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.
bu5hm4n created this revision.Sep 19 2019, 8:57 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/

bu5hm4n requested review of this revision.Sep 19 2019, 8:57 AM
bu5hm4n added a subscriber: zmike.Sep 19 2019, 9:01 AM

I really do not understand why, but this makes the whole test suite fail, because evas starts to magically wait on a lock:

#0  0x00007f3187f1b77c in pthread_cond_timedwait@@GLIBC_2.3.2 () at /usr/lib/libpthread.so.0
#1  0x00007f318856f50e in eina_condition_timedwait (cond=0x7f3188807f40 <evas_thread_queue_condition>, t=2250)
    at ../src/lib/eina/eina_inline_lock_posix.x:439
#2  0x00007f318856f904 in evas_thread_worker_func (data=0x0, thread=139850667869952) at ../src/lib/evas/common/evas_thread_render.c:136
#3  0x00007f3188ed642b in _eina_internal_call (context=0x55cd08ca3cf0) at ../src/lib/eina/eina_thread.c:151
#4  0x00007f3187f1557f in start_thread () at /usr/lib/libpthread.so.0
#5  0x00007f3187e450e3 in clone () at /usr/lib/libc.so.6

@zmike Any ideas what is going on with this ?

Regardless of errors, can you explain me what is supposed to happen now if a user uses NULL, so I can add it to the docs?

Regardless of errors, can you explain me what is supposed to happen now if a user uses NULL, so I can add it to the docs?

NULL with after means equal behavior than efl_pack_end.
NULL with before means equal behavior than efl_pack_before

NULL with after means equal behavior than efl_pack_end.
NULL with before means equal behavior than efl_pack_before

I'm going to assume that you meant efl_pack_begin in the last word and add this to the docs.

This crosses some magical threshold that breaks the current signal handling architecture. Patches I've added to the stack mitigate the issue enough to let the tests pass.

src/lib/elementary/efl_ui_group_item.c
162

Is there a reason this can't just be efl_pack_begin?

183

efl_pack_end?

bu5hm4n added inline comments.Sep 19 2019, 12:19 PM
src/lib/elementary/efl_ui_group_item.c
162

That would be possible but then the _register_item call also needs to be in the if clause, which starts to look like a culpit in future, as you might think that there is a wrong _register_item call in there, if you do not know the code here quite well.

183

Same as above.

@woohyun @Jaehyun_Cho @YOhoho Any comments on that ? Is that okay ?

This revision is now accepted and ready to land.Sep 23 2019, 1:03 PM
Closed by commit rEFL6714c821f787: efl_ui_pack: allow NULL as existing parameter in after and before (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>, committed by zmike). · Explain WhySep 23 2019, 1:16 PM
This revision was automatically updated to reflect the committed changes.