On Fri, Apr 25, 2008 at 10:15 AM, Cedric BAIL <[EMAIL PROTECTED]> wrote:
>
> On Thu, Apr 24, 2008 at 11:15 PM, Gustavo Sverzut Barbieri
> <[EMAIL PROTECTED]> wrote:
> > On Thu, Apr 24, 2008 at 1:57 PM, Cedric BAIL <[EMAIL PROTECTED]> wrote:
> > > On Wed, Apr 23, 2008 at 4:45 PM, The Rasterman Carsten Haitzler
> > > <[EMAIL PROTECTED]> wrote:
> > > > On Tue, 15 Apr 2008 13:15:13 -0300 "Gustavo Sverzut Barbieri"
> > > > <[EMAIL PROTECTED]> babbled:
> > > > > For simplicity, I would just process one message per callback from
> > > > > ecore_fd_main... but we can also use a poll/select inside this
> > > > > function and do what you want, without the need to fcntl to
> > > > > NONBLOCKING.
> > > >
> > > > just read a buffer of messages - if we don't get a complete
> message, store the
> > > > partial one and keep it until the next call to process events then
> complete the
> > > > message fetch.
> > >
> > > Ok. Here is an updated patch. The fd is still nonblocking and on
> > > partial read, it should be able to just restart cleany at a later
> > > time. You will now pass a function pointer so that you can call
> > > whatever kind of code you want (I doubt we really need something else
> > > than evas_object_event_callback). I also make it optional. Some more
> > > comments ?
> >
> > configure: s/build_async/build_async_events/g; make it default on (use
> > "auto" to check for pthread, "yes" would fail without pthread and "no"
> > is... well no).
> >
> > +static pthread_mutex_t _mutex;
> >
> > i think it's safer to just initialize the mutex here using (and remove
> > the destroy call):
> >
> > static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER;
> >
> > evas_async_events_process(void):
> >
> > + if (_fd_read != -1)
> > + do
> >
> > reverse this logic: if (_d_read == -1) return 0;
> > this will avoid the unnecessary nesting and would remove the bug that
> > it have today: "check" might be uninitialized in this function.
> > Actually write the whole function like (more beautiful imho):
> >
> > if (_fd_read == -1)
> > return -1;
> >
> > count = 0;
> > while ((check = read(_fd_read, ((char *)¤t) + size,
> > sizeof(current) - size)) > 0)
> > {
> > size += check;
> > if (size < sizeof(current))
> > continue;
> >
> > if (current.func)
> > current.func(current.target, current.type, current.event_info);
> > size = 0;
> > count++;
> > }
> > if (check < 0) {
> > ...;
> > _fd_read = -1;
> > }
> > return count;
> >
> >
> > For evas_async_events_put, also reverse the logic, I'd write:
> >
> > if (!func) return 0;
> > if (_fd_write == -1) return 0;
> >
> > ...
> > pthread_mutex_lock(&_mutex);
> > do { ... } while ( ... )
> > pthread_mutex_unlock(&_mutex);
> >
> > this would not do unneeded locks and avoid nested code.
> > Also, note that the function return "Evas_Bool" but in the version
> > without BUILD_ASYNC_EVENTS you return -1.
> >
> >
> > Aside these minor, mostly cosmetic, things, it's ready for inclusion :-)
>
> Thanks a lot for taking the time to do the review. Here is a final
> patch, and if nobody oppose, I will commit it during next week.
excellent, commit :-)
/me just waiting for the threaded image loading... ;-)
--
Gustavo Sverzut Barbieri
http://profusion.mobi
Embedded Systems
--------------------------------------
MSN: [EMAIL PROTECTED]
Skype: gsbarbieri
Mobile: +55 (81) 9927 0010
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
