Thread: qsort vs MSVC build
I just tried a rebuild of the MSVC stuff, and got the following error. Any ideas on the best way to fix that? (as you notice, I haven't pulled the very latest cvs so I haven't for the min() fix that's put in now. Just let me know if the rest is also fixed ;-)) //Magnus Build started: Project: postgres, Configuration: Debug|Win32 Compiling... qsort.c 1>.\src\port\qsort.c(53) : warning C4005: 'min' : macro redefinition C:\Program Files\Microsoft Visual Studio 8\VC\include\stdlib.h(808) : see previous definition of 'min' 1>.\src\port\qsort.c(114) : warning C4273: 'qsort' : inconsistent dll linkage C:\Program Files\Microsoft Visual Studio 8\VC\include\stdlib.h(473) : see previous definition of 'qsort' Generate DEF file Not re-generating POSTGRES.DEF, file already exists. Linking... MSVCRTD.lib(MSVCR80D.dll) : error LNK2005: _qsort already defined in qsort.obj Creating library Debug\postgres\postgres.lib and object Debug\postgres\postgr es.exp .\Debug\postgres\postgres.exe : fatal error LNK1169: one or more multiply define d symbols found postgres - 2 error(s), 2 warning(s)
"Magnus Hagander" <mha@sollentuna.net> writes:
> I just tried a rebuild of the MSVC stuff, and got the following error.
> Any ideas on the best way to fix that?
> 1>.\src\port\qsort.c(53) : warning C4005: 'min' : macro redefinition
> C:\Program Files\Microsoft Visual Studio
This is fixed already in HEAD.
> MSVCRTD.lib(MSVCR80D.dll) : error LNK2005: _qsort already defined in
> qsort.obj
Hmm. I've been seeing related complaints on Darwin, but they were just
warnings (about our qsort conflicting with the one in libc).
Is it worth renaming our qsort to pg_qsort to avoid this? (I'd be
inclined to do that via a macro "#define qsort pg_qsort", not by running
around and changing all the code.)
regards, tom lane
> > MSVCRTD.lib(MSVCR80D.dll) : error LNK2005: _qsort already > defined in > > qsort.obj > > Hmm. I've been seeing related complaints on Darwin, but they > were just warnings (about our qsort conflicting with the one in libc). Yeah, seems it works in Mingw, but for some reason it's fatal in MSVC. > Is it worth renaming our qsort to pg_qsort to avoid this? > (I'd be inclined to do that via a macro "#define qsort > pg_qsort", not by running around and changing all the code.) Yeah, I think it is ;-) Just make sure it happens before we pull in stdlib.h, so we don't rename tha tone as well... //Magnus
On Thu, Oct 19, 2006 at 01:56:24PM -0400, Tom Lane wrote: > Is it worth renaming our qsort to pg_qsort to avoid this? (I'd be > inclined to do that via a macro "#define qsort pg_qsort", not by running > around and changing all the code.) Redefining a function that is defined in POSIX and present on most systems seems like a bad idea. Not in the least because ELF linking rules mean that if any library (say libssl) in the backend calls qsort, they'll get the postgresql one, rather than the C library like they expect. That seems fragile to me. The #define would be fine, as long as you make sure it's called after the system headers, otherwise the problem isn't fixed. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
"Magnus Hagander" <mha@sollentuna.net> writes:
>> Is it worth renaming our qsort to pg_qsort to avoid this?
>> (I'd be inclined to do that via a macro "#define qsort
>> pg_qsort", not by running around and changing all the code.)
> Yeah, I think it is ;-) Just make sure it happens before we pull in
> stdlib.h, so we don't rename tha tone as well...
No, we'd want to put the macro after stdlib.h, I should think?
Specifically in port.h, where we play other similar games.
regards, tom lane
> >> Is it worth renaming our qsort to pg_qsort to avoid this? > >> (I'd be inclined to do that via a macro "#define qsort > pg_qsort", not > >> by running around and changing all the code.) > > > Yeah, I think it is ;-) Just make sure it happens before we pull in > > stdlib.h, so we don't rename tha tone as well... > > No, we'd want to put the macro after stdlib.h, I should think? > Specifically in port.h, where we play other similar games. Uh, that's what I meant, I just wrote it backwards. Sorry. //Magnus
On Thu, 2006-10-19 at 13:56 -0400, Tom Lane wrote: > Is it worth renaming our qsort to pg_qsort to avoid this? (I'd be > inclined to do that via a macro "#define qsort pg_qsort", not by running > around and changing all the code.) Why not change each call site? I don't think it would hurt to be clear about the fact that we're calling our own sorting function, not the platform's libc qsort(). -Neil
Neil Conway <neilc@samurai.com> writes:
> On Thu, 2006-10-19 at 13:56 -0400, Tom Lane wrote:
>> Is it worth renaming our qsort to pg_qsort to avoid this? (I'd be
>> inclined to do that via a macro "#define qsort pg_qsort", not by running
>> around and changing all the code.)
> Why not change each call site? I don't think it would hurt to be clear
> about the fact that we're calling our own sorting function, not the
> platform's libc qsort().
I'm concerned about the prospect of someone forgetting to use pg_qsort,
and getting the likely-inferior platform one.
However, the only place where we probably care very much is tuplesort.c,
and that's using qsort_arg now anyway. So plan C might be to drop
port/qsort.c altogether, and just be sure to use qsort_arg anyplace that
we care about not getting the platform one.
regards, tom lane