On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL <[EMAIL PROTECTED]> wrote:
> Hi,
>
>    This patch doesn't break anything at this time :-) It's a
>  standalone feature that just add the possibility to evas to receive
>  events from another thread. It introduce 3 new API.

I'm not sure the fd must be set to non-block, this might cause more
trouble than good. I'd make it blocker for now and handle things
properly using select(), or believe that when the file descriptor is
ready to read (from ecore_fd_main...) we can read everything (ie: no
partial writes, no threads will die with incomplete writes).


>  * Retrieve the fd that need to be monitored by ecore :
>  EAPI int evas_async_events_fd_get();

missing (void)


>  * Process all pending event in the pipe (This function could be called
>  without any event in the pipe) :
>  EAPI int evas_async_events_process();

missing (void), also true for init, shutdown...

also, "do ... while (check == sizeof(current))" logic is wrong, you
should "check += read(..., todo)" and also change where to load the
rest of it.

Actually I would rewrite it as:

int todo = sizeof(current);
char *p = (char *)&current;
do
  {
     int n;
     n = read(_fd_read, p, todo);
     if (n == -1)
       {
         if (errno == EAGAIN || errno == EINTR)
           continue;
         else
           HANDLE_ERROR();
       }
     todo -= n;
     p += n;
  } while (todo > 0)



>  * Push an event inside the pipe from another thread :
>  EAPI Evas_Bool evas_async_events_put(Evas_Object *obj,
>  Evas_Callback_Type type, void *event_info);

Write logic is also wrong.

   &new + offset

given that new is of type Evas_Event_Async is not what you want. This
pointer arithmetic is like vector indexing, so it would be like:

    new[offset]

You need to cast it to some type that is the size of a byte, like char:

    ((char *)&new) + offset

is fine. But I would rewrite it like I did for read.


>  I currently don't have any code using it, but I plan to use this for
>  the coming background loading of image. So please review it.

done :-)


I'm not sure about evas_init() doing this. Nothing against it, but
maybe some purist wouldn't like to have pthread mutexes created.



-- 
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 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
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

Reply via email to