Mony, thanks for all your code updates. While testing the new code, I have spotted a new bug with the Windows libdrizzle.
There are string handling issues that could potentially result in a crash. This bug is due to snprintf type functions (snprintf and vsnprintf) behaving differently on Windows and Linux. Here is a shot summary of the differences between Windows and Linux: - the Windows version does not append a null character at the end if the formatted string is longer than the given size - If the formatted strings is longer than the given size, the Windows version returns -1 This bug can be easily fixed by doing the following: - always set a null character at the the end of the output buffer after calling snprintf type functions - when using the return value from snprintf calls (only in drizzle_set_error from what I can see), the return value should be checked before being added to the output string size. These changes should not affect non-Windows systems so they do not need to go in a "#ifdef _WIN32" section. I am happy to post a new branch with the required fixes, but I just wanted to know first if my thinking was correct or if anyone had better ideas to fix this issue. Regards, Marc On Sun, Apr 3, 2011 at 7:11 PM, Monty Taylor <[email protected]> wrote: > On 03/31/2011 02:04 AM, Marc Isambart wrote: >> Hi, >> >> I have recently compiled libdrizzle on Windows and I have found the >> following potential Windows compilation improvements: >> >> 1) config.h >> The config.h provided in the win32 directory could be improved: >> - PACKAGE_VERSION should be changed to something like "drizzle7" >> instead of "0.8" to match what is done on Linux. >> - DRIZZLED_WIN32_CONFIG_H could be renamed to >> LIBDRIZZLE_WIN32_CONFIG_H as this configuration only applies for >> libdrizzle and not to drizzled >> - the HAVE_* defines could be removed as they are not used by libdrizzle at >> all > > Good catch. Done. > >> 2) pack.c >> Some of the machines I use to compile libdrizzle are limited to Visual >> Studio 2005 (but have the latest Windows SDK) and I get a compilation >> error in pack.c (line 303) because a variable is declared mid-function >> ("uint32_t x= 0;"). >> >> Would it be possible to move the variable declaration to the start of >> the function in order to avoid potential compiler issues (this is the >> only function that has this problem and the change should be easy to >> make). > > Sure. Done. > >> 3) errno.h >> Is there any reason why an LGPL version of errno.h is used when >> compiling on Windows? >> >> Using an LGPL header file could cause potential licensing issues, even >> if the LGPL license is quite vague about using LGPL header files. Here >> is the relevant section taken from LGPL 2.1: >> "When a "work that uses the Library" uses material from a header file >> that is part of the Library, the object code for the work may be a >> derivative work of the Library even though the source code is not. >> Whether this is true is especially significant if the work can be >> linked without the Library, or if the work is itself a library. The >> threshold for this to be true is not precisely defined by law." >> >> Isn't it possible to use the errno.h file provided with Visual Studio >> 2010 instead? > > Well - as you said, some folks are stuck with older visual studios... > but wooha! It sure does turn out that vs2010 doesn't need the errno.h > that I included. GONE! > > (now you get to suggest how I can deal with this for older visual studio > folks :) ) > >> The WSA* error codes do not have the same value as their Posix >> equivalents in the latested errno.h provided by Microsoft (EINPROGRESS >> != WSAEINPROGRESS), so some simple code would be needed just after >> WSAGetLastError() to map the error codes (2 places in conn.c/conn.cc) >> but that should not be too hard to add. >> >> There is already some similar code just after WSAGetLastError(), it >> just needs to be extended to cover all required error codes, for >> example: >> switch(errno) { >> case WSAEINPROGRESS: errno = EINPROGRESS; break; >> case WSAECONNREFUSED: errno = EINPROGRESS; break; >> case WSAENETUNREACH: errno = ENETUNREACH; break; >> case WSAETIMEDOUT: errno = ETIMEDOUT; break; >> case WSAECONNRESET: errno = ECONNRESET; break; >> case WSAEADDRINUSE: errno = EADDRINUSE; break; >> case WSAEOPNOTSUPP: errno = EOPNOTSUPP; break; >> case WSAENOPROTOOPT: errno = ENOPROTOOPT; break; >> } > > Thanks - that's a great idea - and is now done (I mean, it'll be a few > minutes before it's merged and all - but thanks! > > Monty > _______________________________________________ Mailing list: https://launchpad.net/~drizzle-discuss Post to : [email protected] Unsubscribe : https://launchpad.net/~drizzle-discuss More help : https://help.launchpad.net/ListHelp
