-
Notifications
You must be signed in to change notification settings - Fork 129
Conversation
There were a few problems compiling a sample project including "libGeoIP" with Visual Studio on Windows. This patch fixes them by doing a few version checks. The first problem appeared in Visual Studio 2015 and consisted in "snprintf" conflicts and the second problem was an error, actually telling you to add an underline (_) in the front of a few functions such 'fileno'.
This is an artifact of Windows header organization. There's another header you need to include (or possibly a compile-time define which needs to be set, I forget which) that properly exposes snprintf(). This is not the correct resolution of this problem. |
I've got all VS software and I've properly tested with all of them. |
I thought there was some special include file like <tchar.h> or <strsafe.h> which exposes the function directly... Can't remember which. |
I also think the snprintf fix is correct, I did the same thing in order to compile with Visual Studio 2015. Older Visual C++ versions didn't provide snprintf, but Visual C++ 2015 does and doesn't let you redefine it. But could you please guard the warning suppression with a pragma warning(push/pop) so that it doesn't affect user code? One of the most evil things a library can do is to disable warnings in my code. |
Only fix fileno error suppression locally, ensuring no any other user code is being altered by these definitions.
Agreed. Why altering everything when you can just fix that inside libGeoIP? |
This looks mostly good. For the |
If we do so, we also need to do the same for lseek and read. |
#define snprintf _snprintf | ||
|
||
#if defined(_MSC_VER) && _MSC_VER < 1900 | ||
# define snprintf _snprintf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we should move this into GeoIP.c rather than defining it in the header file.
I don't have a strong opinion. Keeping it as is might be safer. It looks like it was deprecated in 2005. |
Change error suppressing method so there is no #pragma warning anymore. Only redefine fileno, read and lseek for Visual Studio 2005 or newer.
This should be reliable as well. Also, transferred snprintf macro to GeoIP.c file. |
Thanks! This looks good. I'll merge as soon as Travis signs off on it. |
Fix compiling issues on Windows - VS
1.6.6 * Replaced usage of deprecated fileno, read, and lseek on Visual Studio 2005+ with their ISO C++ conformant replacements. (Fix by ClaudiuHKS. GitHub #55.) * A warning about using a double as a float was fixed. (Fix by ClaudiuHKS. GitHub #56.) * Fixed segfault when doing a lookup on an empty database. (Fix by NesoK. GitHub #62.) * Fixed a memcheck error from valgrind in the `_check_mtime` function. (Reported by yurivct. GitHub #60.) * Fixed `_check_mtime` to check the return value of `gettimeofday` rather than just assuming it worked.
There were a few problems compiling a sample project including libGeoIP with Visual Studio on Windows.
This patch fixes them by doing a few version checks.
The first problem appeared in Visual Studio 2015 and consisted in snprintf conflicts and the second problem was an error, actually telling you to add an underline _ in the front of a few functions such fileno.