Marc Lehmann, el 15 de enero a las 08:06 me escribiste:
> > your opinion before deciding what to do with them, and of course, I want
> > to know if there is any interest in merging this to the official libev
> > distribution =)
> 
> Yes, there is. Two things are required for inclusion:
> 
> a) it should be sound (i.e. required, within the goals, i.e. relatively lean)
> b) somebody needs to maintain it
> 
> a) is probably given (I haven't had a thorough look :), and would you
> commit to do b)?

Sure.

> > Here's patch that implement this scheme (over the original patch). Even if
> > it's nice to stay close to the C API and to avoid the posibility of
> > instantiating 2 default loops from the root, I think the usage is a little
> > less intuitive in a C++ environment.
> 
> libev already enforces the singleton approach for us, the loop_default
> objects can exist manyfold without us having to worry about it.
> 
> On Mon, Jan 14, 2008 at 06:40:08PM -0200, Leandro Lucarella <[EMAIL 
> PROTECTED]> wrote:
> > * is_default was removed from loop_base, since now the class matches the
> >   type of loop (is_default == dynamic_cast< loop_default* >(loop_ptr)).
> 
> Hmm, the loops are not polymorphic, so the dynamic_cast will not be helpful.
> In any case, is_default == (loop_ == ev_default_loop (0)).

They are in the patch I sent =)
Just for the fork() and destructors.

> > +  enum
> > +  {
> > +    AUTO = EVFLAG_AUTO,
> > +    NOENV = EVFLAG_NOENV,
> > +    FORKCHECK = EVFLAG_FORKCHECK,
> 
> putting all the constants into the ev:: namespace was my original goal
> (when I hoped to keep most of the C api outside the global namespace), but
> I gave up on this.
> 
> In any case, I think there should either be ALL of the symbols available or
> none (or they shouldn't be announced), as remembering whats in ev:: and whats
> not is tedious.
> 
> That was the reason I originally kept out most of the functions outside it,
> too (ev::now is the exception, and it doesn't look well).
> 
> If everything *is* inside ev:: that would be fine, but its quite a bit of
> work. Otherwise, I have no hard feelings either way.

Is not that hard, is a bit of work done once (and it's already done in the
patch ;). So, it's ok for you to expose the entire API in the ev namespace?

> > +  enum how_t
> > +  {
> > +    ONE = EVUNLOOP_ONE,
> > +    ALL = EVUNLOOP_ALL
> 
> Thats different, this way we do gain a bit of type-safety :)

=)

> > +#if EV_MULTIPLICITY
> > +#  define EV_AX  _loop
> > +#  define EV_AX_ _loop,
> > +#else
> > +#  define EV_AX
> > +#  define EV_AX_ ,
> 
> Both EV_AX and EV_AX_ should be empty here, no?

Yeap.

> > +#if EV_MULTIPLICITY
> > +    operator struct ev_loop * () const throw ()
> > +    {
> > +      return _loop;
> 
> for various reasons, I would prefer the "_" at the end, but again, I don't
> care much :)

I'm use to the "_" as a prefix, but I don't care much either and is you
lib, so, I'll use the suffix form ;)

> I wonder why it cannot be simply "loop"? Does it collide with anything?

Yes, with loop_base& loop(). I didn't expose the pointer directly because
I like references when the "ownership" of the pointer is not transfered.
When you receive a reference, you are positive that you can't free it,
when you receive a pointer, you have to check the documentation =)

But if you think is too much trouble, I cant expose the loop (without "_")
pointer directly.

> > +    virtual void fork () throw () = 0;
> 
> fork () might be a global symbol that would collide with this declaration
> (no idea if it atcually causes a problem on existing systems, though).

That would only be true if fork can be a macro symbol. I don't know if
this is actually done by any OS but the POSIX standard don't say anything
about fork() being a macro (or even if it's implementation defined) [1] as
it does with other symbols (FD_XXX for instance [2]).

[1] http://www.opengroup.org/onlinepubs/000095399/functions/fork.html
[2] http://www.opengroup.org/onlinepubs/000095399/functions/select.html

> > +    void set_io_collect_interval (tstamp interval) throw ()
> > +    void set_timeout_collect_interval (tstamp interval) throw ()
> 
> Looks quite complete already :)
> 
> > +    // method callback
> > +    template<class K, void (K::*method)(int)>
> > +    void once (int fd, int events, tstamp timeout, K *object)
> 
> pure fun!
> 
> > +    // simpler function callback
> > +    template<void (*cb)(int)>
> > +    void once (int fd, int events, tstamp timeout)
> > +    {
> > +      once (fd, events, timeout, simpler_func_thunk<cb>);
> > +    }
> 
> Fascinating, shouldn't we add this to the watcher base, too?
> 
> I do wonder how useful it is, I wondered about it myself, but I tend to
> always need the watcher in any real-world callback.
> 
> > +    ~loop_simple () throw ()
> 
> Maybe loop_simpel should rarlly be "loop" if we can get away with it
> without collisions.

Yes, I thought about it too, but with all that "loop" everywhere I didn't
thought it was a great idea (exactly because the high probability of
colission ;). But once the patch is stabilized I can try to rename it to
loop and see what happens...

> > +  inline
> > +  loop_default&
> > +  default_loop (int flags)
> > +  {
> > +    static loop_default l (flags);
> > +    return l;
> > +  }
> 
> I my, I am scared at how horrible code that will generate in each
> translation unit (I know, the same thing is done in ev.h, but its still
> ugly :). Maybe leave it up to the user to declare her own loop_default
> object? Or maybe not.

The problem with that is the lack of a "standard" mechanism to access to
that default loop (for instance, in the watchers contructors to use it as
a default argument).

> >    template<class ev_watcher, class watcher>
> >    struct base : ev_watcher
> >    {
> >      #if EV_MULTIPLICITY
> > -      EV_P;
> > +      loop_base *_loop;
> 
> Thats the only negative thing, as it creates a need to use the ev++.h loop
> wrappers.

And that is wrong because....? I think if you are using ev++.h you'll be
glad to use the loop wrapper =)

> I'd really prefer using the ev_loop *, even if it means slightly more
> annoying code in the watcher.

The problem is not uglier code in watchers. The problem is when you store
an ev_loop*, you can't get back the loop_xxx object (unless you use some
hash to access the object using the loop address =P). I can go back to the
loop_ref approach, but it has its own issues, which I forgot now :S

And I really think it's much more simple and elegant the new approach,
even when it forces you to use the loop wrapper (which you can pass to C
API functions too, so that shouldn't be a huge problem either).

Even more, if the default_loop_ptr is exposed, all virtuality could be
avoided, so no overhead at all will be introduced in the loop wrapper
(well, ok, a check is introduced if EV_MULTIPLICITY is defined in fork()
and ~loop_base(), but I don't think that counts ;), by adding this to
loop_base:

#if EV_MULTIPLICITY
bool is_default()
{
        return loop_ == default_loop_ptr;
}
#endif

void fork()
{
        #if EV_MULTIPLICITY
        if (!is_default())
                ev_loop_fork(loop_);
        else
        #endif
                ev_default_loop(loop_);
}

~loop_base()
{
        #if EV_MULTIPLICITY
        if (!is_default())
                ev_loop_destroy(loop_);
        else
        #endif
                ev_default_destroy(loop_);
}

> Alternatively, how about storing a loop_base in the watcher (note: no "*")
> and accepting both ev_loop * and ev::loop_bases as arguments to set and the
> constructor?
> 
> In fact, since loop_base is comaptible to struct ev_loop one could just
> support ev_loop *'s and internally store a loop_base.
> 
> One would lose the ability to compare loop_base's by address, but a nice
> operator == would do that or somesuch, if needed.

There are several problems with this. First, ev_loop struct is not
available in ev.h =)

Second, I wrapped the loop not only for cosmetical purposes, I like the
automatic destruction =). If you lost the difference between an object and
a raw pointer, then you can't know when to call the ev_xxx_destroy()
(unless we use reference counting, which I think is overkill for this
case).

> > +  inline void sleep (tstamp interval)
> 
> Sleep, again, could collide with a global symbol defined in a header,
> unfortunately.

And again, I don't see POSIX accept sleep being a macro.
http://www.opengroup.org/onlinepubs/000095399/functions/sleep.html

> > -    void start (int fd, int events)
> > +    void start (int fd, int events = READ)
> 
> Do you really think this is such a good idea? Is READ really predominantly
> what people want?

Is what I predominantly want =)

I can remove the default...

> Summary:
> 
> all in all, this is surprisingly straightforward and thin. its really in the
> spirit of ev++.h, I think (or what I define to be that spirit :).

Thanks. Please let me know what do you think about this last comments and
I'll send an updated patch.

-- 
Leandro Lucarella (luca) | Blog colectivo: http://www.mazziblog.com.ar/blog/
----------------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145  104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------------
Estoy de acuerdo en que el hombre es igual a los ojos de Dios..., pero
hay algunos que, siendo negros, hacen uso de lo que no les corresponde.
Eso Dios no lo ve.
        -- Ricardo Vaporeso. Manifestación en Mississippi a favor de
                los blancos, 1923.


_______________________________________________
libev mailing list
libev@lists.schmorp.de
http://lists.schmorp.de/cgi-bin/mailman/listinfo/libev

Reply via email to