BUG #5394: invalid __declspec for PG_MODULE_MAGIC

Started by Vladimir Barzionovabout 16 years ago6 messageshackersbugs
Jump to latest
#1Vladimir Barzionov
snego.barsik@gmail.com
hackersbugs

The following bug has been logged online:

Bug reference: 5394
Logged by: Vladimir Barzionov
Email address: snego.barsik@gmail.com
PostgreSQL version: 8.3.10
Operating system: win32
Description: invalid __declspec for PG_MODULE_MAGIC
Details:

In fmgr.h,
macros PG_MODULE_MAGIC, PG_FUNCTION_INFO_V1.. are declared with incorrect
"storage-class". It uses PGDLLIMPORT for declare __declspec().

In general for building external DLL module, PGDLLIMPORT must be
__declspec(dllimport). As it is.

However in PG_MAGIC_* macro, storage class for function must be
__declspec(dllexport).

Same problem was already discussed for example here
http://dbaspot.com/forums/postgresql/393683-re-general-custom-c-function-pal
loc-broken.html

Looks like the simplest way for correcting the issue is declaring additional
macro (something like PGMODULEEXPORT)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Vladimir Barzionov (#1)
hackersbugs
Re: BUG #5394: invalid __declspec for PG_MODULE_MAGIC

"Vladimir Barzionov" <snego.barsik@gmail.com> writes:

In fmgr.h,
macros PG_MODULE_MAGIC, PG_FUNCTION_INFO_V1.. are declared with incorrect
"storage-class". It uses PGDLLIMPORT for declare __declspec().

In general for building external DLL module, PGDLLIMPORT must be
__declspec(dllimport). As it is.

However in PG_MAGIC_* macro, storage class for function must be
__declspec(dllexport).

Same problem was already discussed for example here
http://dbaspot.com/forums/postgresql/393683-re-general-custom-c-function-pal
loc-broken.html

Looks like the simplest way for correcting the issue is declaring additional
macro (something like PGMODULEEXPORT)

If you want us to change this, you need to give a specific example of
what's going wrong for you. Just asserting that it's wrong adds nothing
whatsoever to the previous discussions.

regards, tom lane

#3ITAGAKI Takahiro
itagaki.takahiro@oss.ntt.co.jp
In reply to: Vladimir Barzionov (#1)
hackersbugs
Re: BUG #5394: invalid declspec for PG_MODULE_MAGIC

"Vladimir Barzionov" <snego.barsik@gmail.com> wrote:

Same problem was already discussed for example here
http://dbaspot.com/forums/postgresql/393683-re-general-custom-c-function-palloc-broken.html

Looks like the simplest way for correcting the issue is declaring additional
macro (something like PGMODULEEXPORT)

Sure, I agree it is a longstanding bug in PostgreSQL. Developers who use
MSVC (not mingw) always encounter the bug; machines in the buildfarm can
build Windows binaries just because they have non-standard equipments.

A patch attached. The name of "PGMODULEEXPORT" might be arguable.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center

Attachments:

PGMODULEEXPORT.patchapplication/octet-stream; name=PGMODULEEXPORT.patchDownload+9-2
#4Magnus Hagander
magnus@hagander.net
In reply to: ITAGAKI Takahiro (#3)
hackersbugs
Re: BUG #5394: invalid declspec for PG_MODULE_MAGIC

On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

"Vladimir Barzionov" <snego.barsik@gmail.com> wrote:

Same problem was already discussed for example here
http://dbaspot.com/forums/postgresql/393683-re-general-custom-c-function-palloc-broken.html

Looks like the simplest way for correcting the issue is declaring additional
macro (something like PGMODULEEXPORT)

Sure, I agree it is a longstanding bug in PostgreSQL. Developers who use
MSVC (not mingw) always encounter the bug; machines in the buildfarm can
build Windows binaries just because they have non-standard equipments.

A patch attached. The name of "PGMODULEEXPORT" might be arguable.

I agree with this in principle, but won't this break every single
add-on module out there that supports Win32?

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Magnus Hagander (#4)
hackersbugs
Re: BUG #5394: invalid declspec for PG_MODULE_MAGIC

Magnus Hagander <magnus@hagander.net> writes:

On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

A patch attached. The name of "PGMODULEEXPORT" might be arguable.

I agree with this in principle, but won't this break every single
add-on module out there that supports Win32?

The patch didn't touch the contrib modules, so why would it break
third-party sources?

regards, tom lane

#6Magnus Hagander
magnus@hagander.net
In reply to: Tom Lane (#5)
hackersbugs
Re: BUG #5394: invalid declspec for PG_MODULE_MAGIC

On Tue, Apr 6, 2010 at 21:55, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Magnus Hagander <magnus@hagander.net> writes:

On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
<itagaki.takahiro@oss.ntt.co.jp> wrote:

A patch attached. The name of "PGMODULEEXPORT" might be arguable.

I agree with this in principle, but won't this break every single
add-on module out there that supports Win32?

The patch didn't touch the contrib modules, so why would it break
third-party sources?

Oh, d'oh. I was clearly too tired when reading that thing. I was
thinking we required changes in the contrib modules/third party
modules, but they don't actually use DLLEXPORT, do they?

In that case, that argument clearly falls, and I'm in favour of the
patch :-) Just make sure you test it on both current and semi-old
versions of mingw, they tend to not always act the same way. Or just
get it in and the buildfarm can figure that part out for us.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/