On Tue, 20 Mar 2018 10:06:43 -0300 Gustavo Sverzut Barbieri
<barbi...@gmail.com> said:
> > static void
> > _exe_exit_eval(Eo *obj, Efl_Exe_Data *pd)
> > {
> > if ((pd->fd.out == -1) && /*(pd->fd.in == -1) &&*/
> > (pd->fd.exited_read == -1) && (!pd->exit_called))
> > {
> > - Eina_Future *job;
> > - Eo *loop = efl_provider_find(obj, EFL_LOOP_CLASS);
> > -
> > pd->exit_called = EINA_TRUE;
> > - efl_ref(obj);
> > - job = eina_future_then(efl_loop_job(loop), _efl_loop_task_exit,
> > obj);
> > - efl_future_Eina_FutureXXX_then(obj, job);
> > + if (pd->promise)
> > + {
> > + int exit_code = efl_task_exit_code_get(obj);
> > + if (exit_code != 0) eina_promise_reject(pd->promise,
> > exit_code + 1000000);
> > + else eina_promise_resolve(pd->promise,
> > eina_value_int_init(0));
>
> I said I'd give up... but seeing this makes me sad.
>
> in two lines you: misused the Eina_Error AND missed the purpose of resolve.
>
> I hoped when you looked at it, written as code, you'd realize the
> mistake... but seems not... so I'm pointing here.
>
> eina_error conversion to string (which will be helpful to high level
> languages) is missed, "+ 1000000" is an error that is not
> registered...
>
> OTOH success is always ZERO, so there is no value... IF that would be
> right, then it should be a void promise (carries no value).
>
> but my hope is that you'll simply move this to
> eina_promise_resolve(pd->promise, eina_value_int_init(exit_code))
>
> and listen to my advice that is to move the "!= 0" *OUTSIDE* of this
> function, you're mixing roles :-/
eh? we only have 1 value to know of success or failure. an int. 0 == success.
anything else is failure. that's precisely what i did right there.
take a look at efl_io_manager.c ... go on. this is EXACTLY[ what it does too.
the same examples of using promises tat you want me to look at do exactly this.
just look at _file_stat_done_cb()
it does exactly the same thing:
static void
_file_stat_done_cb(void *data, Eio_File *handle EINA_UNUSED, const Eina_Stat
*st) {
const Eina_Value_Struct value = { _eina_stat_desc(), (void*) st };
Eina_Promise *p = data;
Eina_Value r = EINA_VALUE_EMPTY;
if (!eina_value_setup(&r, EINA_VALUE_TYPE_STRUCT))
goto on_error;
if (!eina_value_pset(&r, &value))
goto on_error;
eina_promise_resolve(p, r);
return ;
on_error:
eina_value_flush(&r);
eina_promise_reject(p, eina_error_get());
}
yes. in this case on success the value is the struct data for the state. for an
exe i don't have anything. it's a 0. that's it. that's all i have.
that is one reason why i kept saying that an object is the right thing to pass
here, not an value... but well. i'm obviously so incredibly wrong there so i
did exactly what you said to do and passed and int.
> at least you could see that eina promise is not that complex... the
> only issue remaining is this one I'm pointing you.
it's still a pain. and also a pain to use with different callback signature to
events.
> I know we don't have the eina_future_cb_compare(), but adding that
> will be usable everywhere and make your efl_task interface more
> powerful.
you didn't listen. then how do you determine the kind of error from an
executable that returns != 0 for the kind of error vs ECANCELED. ECANCELED on
linux at least is 125. that is a perfectly possible error exit code from an
executable. i shifted the error codes up by 1000000 *IF* the error code != 0.
if it's zero no error type is passed. the int type is passed instead with is
always 0. there is no other thing to return except always 0 because you
absolutely insist that promises are values not objects and they absolutely must
be a value, so there is the return code. voila. it's an int and it's always 0.
that's how it's defined for executables.
0 == success for executables. anything else is an error. that is precisely how
the promise is exposing it. i wasn't the one who designed promises to only
carry a single error value. there isn't any sensible way of converting error
CODES to strings because every executable will be different and it's on a case
by case basis. the same applies to threads.
>
>
>
> > @@ -531,7 +533,7 @@ _efl_exe_efl_task_run(Eo *obj EINA_UNUSED, Efl_Exe_Data
> > *pd) _exec(cmd, pd->flags);
> > // we couldn't exec... uh oh. HAAAAAAAALP!
> > exit(-122);
> > - return EINA_FALSE;
> > + return NULL;
>
> this is one place you could notify the main process and cause it to
> reject with the errno.
i don't see why. it's after the exec.. and it's after an exit. i put the return
there just to ensure we don't have warnings for "no return value for function".
it's after a fork in the child too... so how is this future return going to
work then? i'd need an extra side channel pipe just to pass this side band
data. i haven't done that. i don't plan to because the cost (2 more fd's per
executed process regardless if you want the stdin/out or not) is not worth it
just to fine-grain return an error code that exec failed or that fork failed vs
the actual process failing.
--
------------- Codito, ergo sum - "I code, therefore I am" --------------
Carsten Haitzler - ras...@rasterman.com
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
