First, I would like to make a happy report that I just got the latest eventxx & libev fixes (patching the latest cvs fixes into the libev-1.3e pre-release), built on OSX, and ran all but 2 tests (already known problems) OK. Thanks for the good work!
Marc, I took a quick scan of the latest diffs and was pleased-especially with the conditional include file macro. I hadn't expected this would work so well... Now, I would like to add a "small" remark about the different versions of the event.h API in libev & libevent. eventxx is using its own check for ev_flags & EVLIST_ACTIVE as a guard before calling event_priority_set(). However, libev does not have these 2 definitions. Even if libev did support priority, this would be a (small) issue. I think the easiest solution is for eventxx to remove this check, since it is already done by event_priority_set(). CB On Nov 9, 2007 8:02 PM, Leandro Lucarella <[EMAIL PROTECTED]> wrote: > Marc Lehmann wrote: > > On Fri, Nov 09, 2007 at 12:47:03AM +0100, Chris Brody-GMail <[EMAIL > > PROTECTED]> wrote: > >> tests on OSX. In addition, all test from eventxx were working except > >> for bench.cpp, which uses a strange copy constructor, and priority > > > > I didn't find anything related to a copy constructor on glancing over the > > source, but eventxx > > (http://git.llucax.com.ar/?p=software/eventxx.git;a=blob;f=eventxx;h=7b4d950d087255d735fb7a550f32d8b0a8fd0d21;hb=HEAD) > > has some deep problems: > > > > 203 int r = static_cast< int >(t1) | static_cast< int >(t2); > > 204 int* pr = &r; // Avoid some weird warning about dereferencing > > 205 // type-punned pointer will break > > strict-aliasing rules > > 206 return *reinterpret_cast< type* >(pr); > > > > here it assumes that "type" has the same representation as an int, which > > isn't true in general but at least common enough. > > About this, it's really not very likely to have an enum that is not > stored as an int, but I checked the C++ standard you are right, an enum > is not an integral type, but it can be promoted to one, so all that > horrible, horrible reinterpret_cast stuff should not be necessary. I > tried it and compiles and passes the tests, so I think maybe it was a > bug in the gcc versiones I used when I tried to make the simple cast > from/to type and didn't worked (the reason for the ugly hack). > > Anyway, this is fixed now in the git repo. > > > and the warning is > > actually hinting at another problem: the compiler may assume that the type > > * does not point to pr by language rules (and newer gcc versions take > > advantage of this in more and more cases). > > I could never understand what that type punned means (and I still don't, > so if you can give me another pointer about what is all about, I'll be > very thankful =). > > > much worse: > > > > 325 short* pev = &ev; // Avoid some weird warning > > about > > 326 // dereferencing type-punned > > pointer > > 327 // will break > > strict-aliasing rules > > 328 handler(fd, *reinterpret_cast< type* >(pev)); > > > > here it assumes that "type" (which afaics is the same enum as above) > > has the same representation as a short (ev is actually short), and the > > aliasing rule applies too. > > > > it cannot be both int and short, in general. > > > > most likely, this works only by chance and only on little-endian > > architectures. > > Yes, you are right, I should cast the pointer to a temporary int first > and then do a regular cast from int to short, but this is unnecessary > since the direct conversion from type enum to short works too (as an > enum can be promoted to any integral type in which it can fit). > > Fixed too. > > > regarding copy constructors, if used, and I don't see where in bench.cpp > > it would use them, but it is entirely possible, it would break stuff, as > > the compiler-synthesized copy constructor will not do the right thing for > > either libevent or libev for active watchers. the best way would be to > > make it private without implementing it, or using stop/start. > > dispatcher copy-constructor and operator= were hidden too to avoid > accidental uses as it has no sense to copy it. > > Thanks for your review and for the time you took to write this down. > > -- > Leandro Lucarella > Integratech S.A. > 4571-5252 > _______________________________________________ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users
