On Wed, Jul 28, 2010 at 8:18 PM, Gustavo Sverzut Barbieri <
barbi...@profusion.mobi> wrote:
> On Wed, Jul 28, 2010 at 10:05 PM, Stephen Houston <unixti...@gmail.com>
> wrote:
> > On Wed, Jul 28, 2010 at 6:33 PM, Gustavo Sverzut Barbieri
> > <barbi...@profusion.mobi> wrote:
> >>
> >> On Wed, Jul 28, 2010 at 7:12 PM, Enlightenment SVN
> >> <no-re...@enlightenment.org> wrote:
> >> > Log:
> >> > Add the ability to perform standard IO operations on a file in a
> >> > thread.
> >> > +#define EIO_FILE_MOD_TIME 1
> >> > +#define EIO_FILE_SIZE 2
> >> > +#define EIO_FILE_EXISTS 4
> >> > +#define EIO_FILE_IS_DIR 8
> >> > +#define EIO_FILE_CAN_READ 16
> >> > +#define EIO_FILE_CAN_WRITE 32
> >> > +#define EIO_FILE_CAN_EXECUTE 64
> >>
> >> You should provide some EIO_FILE_STAT, to return the whole stat
> >> result, maybe that is more useful than doing it separated for size,
> >> exists... you can leave what you did, but also provide the generic
> >> function as people will consult more than one property at time.
> >
> > I left that out because you can specify as many of the operations as you
> > want anyway.
>
> You force people to define many callbacks, or one with a big switch
> that is called multiple times. Still you don't provide other stat
> information that may be useful, like owner, mode...
>
>
> >> Also, why not make it an enum?
> >
> > It was an enum originally, but cedric thought bit flags would be better.
>
> BAD cedric ;-)
>
>
>
> >> > +static void
> >> > +_eio_file_op_end(void *data)
> >> > +{
> >> > + Eio_File_Op *async;
> >> > +
> >> > + async = data;
> >>
> >> Just do Eio_File_Op *async = data, it is smaller and makes sense as
> >> you're just converting the parameter.
> >>
> > Ahhh the E formatting... :(
>
> strictly it is not e formatting, raster uses what you did, but it just
> adds lines to code, and leads to easier "unused variable" errors for
> no good.
>
>
>
> >> > + if (!async->exists)
> >> > + {
> >> > + if (async->flags & EIO_FILE_EXISTS == EIO_FILE_EXISTS)
> >> > + async->main_cb((void *)EINA_FALSE, EIO_FILE_EXISTS, data);
> >> > + ecore_thread_cancel(async->common.thread);
> >> > + return;
> >> > + }
> >>
>
Right here. If the file doesn't exists(this is why we check here and if the
user wanted to know if it exists return FALSE) then the thread is canceled
and the users error callback triggered. Therefore CAN_READ, CAN_WRITE, and
CAN_EXECUTE, and any other define will receive the error cb if the file does
not exist.
> >> I don't like this, you don't return anything for CAN_READ, CAN_WRITE
> >> and others in the case it does not exists? Not even an error? IMO
> >> the user should always get called back, with an error whenever
> >> appropriate. Or call error_cb if there were requirement not fulfilled.
> >>
> >> also a bug there: the last parameter is USER data, not the one you got
> >> from the callback ("async"), you really want: async->common.data
> >> instead!
> >>
> > Actually CAN_READ CAN_WRITE and CAN_EXECUTE will return error if the file
> > doesn't exist (see if (!async->exists)). Otherwise is EINA_TRUE or
> > EINA_FALSE as to whether it can do those. Good catch on the bug.
>
> I fail to see how they will return error, if the file did not exist,
> then it will report for EIO_FILE_EXISTS and return :-/
>
>
>
>
> >> > + if (async->flags & EIO_FILE_EXISTS == EIO_FILE_EXISTS)
> >> > + async->main_cb((void *)EINA_TRUE, EIO_FILE_EXISTS, data);
> >>
> >> this is ugly and took me a while to notice why you did the EINA_TRUE
> >> constant here... it was because of the other test before. And really,
> >> why not call this one first?
> >>
> > I'm not sure what you are getting at here? It returns FALSE if it doesn't
> > exists, and TRUE if it does. The reason the FALSE is called first is
> > because the app will seg if any other operations are done on a file that
> > doesn't exists.
>
> I did notice that, but it took some time as at first it looked like a
> bug "he should use the variable and not a constant!", then I noticed
> the other case was handled before.
>
> what I meant to call this one first, is that we should dispatch the
> EIO_FILE_EXISTS before dispatching CAN_READ, CAN_WRITE...
>
>
>
> --
> Gustavo Sverzut Barbieri
> http://profusion.mobi embedded systems
> --------------------------------------
> MSN: barbi...@gmail.com
> Skype: gsbarbieri
> Mobile: +55 (19) 9225-2202
>
------------------------------------------------------------------------------
The Palm PDK Hot Apps Program offers developers who use the
Plug-In Development Kit to bring their C/C++ apps to Palm for a share
of $1 Million in cash or HP Products. Visit us here for more details:
http://p.sf.net/sfu/dev2dev-palm
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
