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