On Mon, Jul 22, 2013 at 8:55 AM, Carsten Haitzler <ras...@rasterman.com> wrote:
> On Sun, 21 Jul 2013 15:18:02 +0200 Jérémy Zurcher <jer...@asynk.ch> said:
>> Hello from my holiday in France (for the frenchies who know lofofora ;)

Enjoy !

>> On Friday 12 July 2013  15:42, Carsten Haitzler wrote :
>> > On Mon, 08 Jul 2013 18:00:15 +0100 Tom Hacohen <tom.haco...@samsung.com>
>> > said:
>> >
>> > ok. looked at eo2test.c
>> >
>> >    eo2_do(obj,
>> >          a = inst_func(eo_o, 32);
>> >          inst_func(eo_o, 10);
>> >          b = inst_func(eo_o, 50);
>> >          );
>> >
>> > first... passing in eo_o... should probably go away. ie
>> >
>> >    eo2_do(obj,
>> >          a = inst_func(32);
>> >          inst_func(10);
>> >          b = inst_func(50);
>> >          );
>>
>> the declaration is
>> EAPI int inst_func_set(eo2_a, int a, int b);
>>
>> expanded to
>> EAPI int inst_func_set( _Eo *obj, Eo *objid, int a, int b);
>>
>> what I first proposed was using a second macro
>>     eo2_do(obj,
>>           sum = eo_call(inst_func_set, 32, 12);
>>           a = eo_call(inst_func_get_a);
>>           );
>>
>> would it be better ?
>> (Tom, I don't love it, that's just handy ;)
>
> i dont see why? make inst_func() a macro that defines to inst_func_real
> (__cur_obj, ...) eo2_do() defines a local var for __cur_obj when obj is
> looked-up (so lookup happens only once per do block).
>
>> > secondly, having a = xxxx; with ; too.. is going to start encouraging
>> > people to do really bad things like:
>> >
>> >    eo2_do(obj,
>> >          if (inst_func(32) != 22) return;
>> >          inst_func(10);
>> >          b = inst_func(50);
>> >          );
>> >
>> > and all sorts of hell is going to break loose here. i don't believe that as
>> > a result of returns we can now sensibly handle method errors from an 
>> > eo_do()
>> > batch. if we are using returns for errors we SHOULD have something like:
>>
>> well the rule is really simple to document and explain:
>>   use break not return in e eo2_do() construct
>
> next problem. if we ref() on do start and unref() on end of do... this just
> broke it. any form of breaking out will break stuff badly (break, return,
> continue).

Not break. It is easy to make it work without using macro. We just
embedded it into a do { } while (0); loop and the break will work just
fine. The problem will be that the semantic of continue will not be
related to a loop where eo_do is running, but to eo_do code itself. I
would just strongly advise in the documentation to not use those
trick.

>> > Eo_Err err;
>> >
>> > err = eo_do(obj,
>> >             a = inst_func(32);
>> >             inst_func(10);
>> >             b = inst_func(50);
>> >             );
>> > if (err.error == inst_func1_id) return; // inst_func1 method id is what
>> > failed
>>
>> we have no more function id in the latest prototype,
>> but this disallow EAPI static inline func();
>>
>> I'm waiting for comments from Tom and others about this.
>
> static inline is  incompatible with things like method overriding at
> runtime anyway :) but still... imho the "now we have returns we can get 
> errors"
> is a bogus claim, because of the previous point... you can get.. but you cant
> DO anything useful (return, break, continue)... UNLESS you provide a macro 
> that
> cleanly aborts the do block (example - #define eo2_break() 
> _eo_unref(__cur_obj);
> break). i was just showing how errors could be done very simply with the
> current eo api by returning an eo_err struct, rather than a single bool.
>
>> > if (err.failedop == 2) return; // in a batch, operations are counted, 0, 1,
>> > 2 // so op id 2 (last one) failed here
>> > if (err.type == EO_ERR_INVALID_VALUE) return; // can put err inside a type
>> > field.
>> >
>> > note that this is using an error return as a struct, not a basic type (int,
>> > pointer, char, etc.), and this is perfectly valid and correct in basic c.
>> > it's not an extension. we COULD pass in a ptr to the err struct too instead
>> > of using a return.
>> >
>> > but... in the end the error is returned and deal with OUTSIDE of the 
>> > eo_do()
>> > block. so we guarantee to always exit the eo_do() block at the end safely
>> > and handle refcounts properly. if people start putting arbitrary code
>> > inside the block with returns and what not.. compilers will allow this in
>> > eo2... and we will have problems. big ones. i can smell it right now. so
>> > i'd say dont allow ; s and arbitrary code inside. use , i.e.
>> >
>> > Eo_Err err;
>> >
>> > err = eo_do(obj,
>> >             a = inst_func(32),
>> >             inst_func(10),
>> >             b = inst_func(50),
>> >             );
>> >
>> > ... needs some looking into maybe ...
>>
>> what does 'guarantee' and 'don't allow' mean ? I think no more than words in
>> an API doc. if users want to try wrong construction they still will be able
>> to. (sure eo_do() is stronger in that case, compiler does not accept code in
>> varargs !!) So what matters is which one is the simplest to use and to
>> document. for me it's clearly 'do not use return' btw what users and us will
>> do is not much than eo2_do(obj,
>>         ret = eo_call(do_some, 133, 254);
>>         if(!eo_call(try_this,435)) break;
>>         ret = eo_call(all_perfect);
>>         );
>>
>> no need to explain users how to handle the error code, no need for us to
>> waste precious time checking last call status to know if we must break eo2_do
>> (…)
>
> break kills the ref/unref pairs... (we ref so any calls that may directly or
> indirectly delete an object inside don't cause the rest of he ops to crash and
> obj is cleaned up then at the end of the do block).

Not in that case, no problem there ! :-)

>> > my other reservation is that e are exposing the pointer to an eo obj TO the
>> > application code.. and the objid stuff in eo was to try isolate this. this
>> > now leaks it out into the app... allowing for abuse and misuse. that is a
>> > downside of this.
>>
>> yes, I too dislike this, but don't think this is that harmfull.
>> it's an opaque type, what could users do with it, free it ??
>> bloody bastards may the gods punish them I'would say!
>
> well it's more that it's now within the stack frame of a caller func. that
> means its there... the minimum we should do is set it to NULL at the end of 
> the
> do block so they can't see it.
>
>> btw I tried to use an object context instead, see devs/jeyzu/eo2
>> it looks like this (eo2_do() expanded):
>>   ctx = eo2_do_start(obj_id);
>>   my_eapi_func(ctx, args);
>>   ...
>>   eo2_do_end(ctx);
>>
>> the goodies are:
>>   - no more object pointer leaking in user land
>>   - detect a missing eo2_do_end() on the next call to eo2_do_start()
>>   - cache the object data pointer
>>
>> ctx is not thread safe but could be
>
> ultimately we'd have to make it threadsafe. as long as ctx is hidden from the
> user code and they dont have to keep typing it in... :) the problem now comes
> with making this threadsafe and not hurting performance-wise. with the current
> eo_do() we can just mutex lock() and unlock() objs on entry to eo_do() and on
> exit of eo_do(). ... stack of course inside eo is private per thread so we're
> good. to avoid locks on ctx it'd have to use thread local storage - and then
> that begs the q... what to do when that thread is ended? how to delete tls?

Using lock should not impact speed to much, but we have benchmark to
test that, so let's see !
--
Cedric BAIL

------------------------------------------------------------------------------
See everything from the browser to the database with AppDynamics
Get end-to-end visibility with application monitoring from AppDynamics
Isolate bottlenecks and diagnose root cause in seconds.
Start your free trial of AppDynamics Pro today!
http://pubads.g.doubleclick.net/gampad/clk?id=48808831&iu=/4140/ostg.clktrk
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to