qsort vs MSVC build

Started by Magnus Haganderabout 19 years ago8 messages
#1Magnus Hagander
mha@sollentuna.net

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)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#1)
Re: qsort vs MSVC build

"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

#3Magnus Hagander
mha@sollentuna.net
In reply to: Tom Lane (#2)
Re: qsort vs MSVC build

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

#4Martijn van Oosterhout
kleptog@svana.org
In reply to: Tom Lane (#2)
Re: qsort vs MSVC build

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/

Show quoted text

From each according to his ability. To each according to his ability to litigate.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#3)
Re: qsort vs MSVC build

"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

#6Magnus Hagander
mha@sollentuna.net
In reply to: Tom Lane (#5)
Re: qsort vs MSVC build

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

#7Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#2)
Re: qsort vs MSVC build

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#7)
Re: qsort vs MSVC build

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