Re: BUG #5394: invalid declspec for PG_MODULE_MAGIC

The following bug has been logged online:

Bug reference: 5394
Logged by: Vladimir Barzionov
Email address: snego(dot)barsik(at)gmail(dot)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)

"Vladimir Barzionov" <snego(dot)barsik(at)gmail(dot)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

"Vladimir Barzionov" <snego(dot)barsik(at)gmail(dot)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

On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
<itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
>
> "Vladimir Barzionov" <snego(dot)barsik(at)gmail(dot)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/

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
> <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)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

On Tue, Apr 6, 2010 at 21:55, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Mon, Mar 29, 2010 at 11:47 AM, Takahiro Itagaki
>> <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)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/