On 23/10/14 14:19, Chris Michael wrote: > On 10/23/2014 09:05 AM, Tom Hacohen wrote: >> On 23/10/14 14:03, Srivardhan Hebbar wrote: >>> devilhorns pushed a commit to branch master. >>> >>> http://git.enlightenment.org/core/efl.git/commit/?id=005cdf359d185c620944c70716d629062a4f6371 >>> >>> commit 005cdf359d185c620944c70716d629062a4f6371 >>> Author: Srivardhan Hebbar <[email protected]> >>> Date: Thu Oct 23 09:02:11 2014 -0400 >>> >>> ecore_wayland: Making _ecore_wl_init_count not to go below zero in >>> shutdown. >>> >>> Summary: >>> If _ecore_wl_init_count goes below zero, then there would be problem >>> if someone calls ecore_wl_shutdown 1st and then ecore_wl_init later. So >>> fixing this issue in ecore_wl_shutdown. >>> >>> @fix >>> >>> Signed-off-by: Srivardhan Hebbar <[email protected]> >>> >>> Reviewers: devilhorns >>> >>> Reviewed By: devilhorns >>> >>> Subscribers: cedric >>> >>> Differential Revision: https://phab.enlightenment.org/D1571 >>> --- >>> src/lib/ecore_wayland/ecore_wl.c | 1 + >>> 1 file changed, 1 insertion(+) >>> >>> diff --git a/src/lib/ecore_wayland/ecore_wl.c >>> b/src/lib/ecore_wayland/ecore_wl.c >>> index ba00b43..20c6d3b 100644 >>> --- a/src/lib/ecore_wayland/ecore_wl.c >>> +++ b/src/lib/ecore_wayland/ecore_wl.c >>> @@ -448,6 +448,7 @@ _ecore_wl_shutdown(Eina_Bool close) >>> { >>> LOGFN(__FILE__, __LINE__, __FUNCTION__); >>> >>> + if (_ecore_wl_init_count < 1) return 0; >>> if (--_ecore_wl_init_count != 0) return _ecore_wl_init_count; >>> if (!_ecore_wl_disp) return _ecore_wl_init_count; >>> >>> >> >> This is not a good fix. It just hides the bug below the carpet. You >> should also print an error message saying that it got below 0, so we >> won't miss this issue again, and you should strive to find the reason >> why it gets below 0, i.e who's shutting it down without initializing it. >> > > Ideally, yes we should find out who is shutting down without init ... > but since these are libraries, we cannot control every app which uses > them. So it could end up being a rogue application which is not doing > things properly ... Hebbar just suggested we use an assert here to die > so the bad app knows it's bad ... Personally, I am not a fan of asserts > but I think for this case it may be fine ... Thoughts ??
Not an assert, an error message ERR() would be good. I know it's a lib, I thought you found something specific that triggered it, and that's why you've fixed it. Anyhow, I'd add an error message. -- Tom. ------------------------------------------------------------------------------ _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
