On Wed, Apr 23, 2008 at 05:34:04PM +0400, Eugene 'HMage' Bujak wrote: > Hi, > > I want to submit a more complete patch for VC6. > > This one makes all elements of the code compilable and this code is > being used on a production-level project at work. > > The patch is in attachment. Compiles and works under VC6. > * signal_test works. > * timer_test works. > * event_test fails due to the reason that win32 select() can't work on > anything but network sockets.
Thanks for the patch; it's good stuff! I'd like to apply it, but there are a few small issues or things that could be done better in order to work well on *non*-vc6 projects. > * INPUT and OUTPUT are defined in platform SDK, so I had to redefine > them as EVRPC_INPUT and EVRPC_OUTPUT. We can't do this change in mainline libevent; it'll break all existing evprc users that use the old names. Maybe there's some way we we can make the old names available on non-vc code, but deprecated in favor of the EVRPC names? something like this: enum EVRPC_HOOK_TYPE { EVRPC_INPUT, EVPRC_OUTPUT, }; #ifndef _WIN32 #define INPUT EVRPC_INPUT #define OUTPUT EVRPC_OUTPUT #endif [...] > * Winsock library needs to be initialized and freed explicitly on win32. This is true, but it's not libevent's job to do it. If we make this change so that libevent starts initializing winsock, we'll break all existing programs that use libevent and know to initialize winsock themselves. It's quite reasonable to call some sockets code before calling event_init, but this part of the patch makes that result in double-initializing winsock. Also, I think this change will double-initialize winsock on all programs that use multiple event bases, and double-shutdown winsock whenever the bases are closed on those programs. [...] > -/* Define to appropriate substitue if compiler doesnt have __func__ */ > -#if defined(_MSC_VER) && _MSC_VER < 1300 > +/* Define to appropriate substitute if compiler doesn't have __func__ */ > +#ifdef _MSC_VER > #define __func__ "??" I thought some versions of VC *did* have an acceptable version of __func__ or a replacement for it. If that's true, this will break those other VCs. What is _MSC_VER defined to on VC6? Should the test be something other than _MSC_VER < 1300? > diff -ur libevent-1.4.3-stable/event-config.h > libevent-1.4.3-stable-win32/event-config.h > --- libevent-1.4.3-stable/event-config.h Wed Apr 23 17:31:14 2008 > +++ libevent-1.4.3-stable-win32/event-config.h Wed Apr 23 17:31:01 2008 > @@ -51,7 +51,9 @@ > #define _EVENT_HAVE_INET_NTOP 1 > > /* Define to 1 if you have the <inttypes.h> header file. */ > +#ifndef _MSC_VER // MSVC doesn't have that > #define _EVENT_HAVE_INTTYPES_H 1 > +#endif This approach is the wrong way to make event-config.h work. On all toolsets besides VC, the file is automatically generated from config.h, which itself is automatically generated by the autoconf script. The right approach for VC is just to have a separate config.h and event-config.h script that work, ideally in the WIN32-Code directory, and set up the compiler's search path so those get used instead of any other ones that might be kicking around. [...] > +#ifdef _WIN32 // MSVC doesn't have that FWIW, many older C compilers don't allow C++-style comments: they weren't added to the C standard till the C99 standard. We need libevent to build on older compilers, so we can't add C++-style comments to libevent. Otherwise, though, this looks like solid work, and I'm quite glad that somebody has a copy of VC6 and the willingness to do it! If you can send in a revised patch, that'd be great. Otherwise, I'll try to hand-apply it with modifications as noted above, but since my x86 box is down at the moment, I can't fire up windows to test it out now, and things might go badly. many thanks, -- Nick Mathewson _______________________________________________ Libevent-users mailing list Libevent-users@monkey.org http://monkeymail.org/mailman/listinfo/libevent-users