Re: Custom C function - is palloc broken?

Nathan Thatcher wrote:
> So what options does that leave all of us who need to compile and run
> our custom C functions in Windows?

After a bit of sleep and with the advantage of a now-working brain I've
got it working, at least with 8.3 .

The problem was with DLL linkage, especially the CurrentMemoryContext
global variable. I was defining BUILDING_DLL and using the PGDLLIMPORT
macro to export the copytext() function - but this was of course causing
everything in the Pg headers to be declared __declspec(dllexport)
instead of __declspec(dllimport) . AFAIK this is survivable for
functions, which will use a slower call thunk instead, but not for
exported variables.

As a result, MemoryContextAlloc(CurrentMemoryContext, (sz)) was being
called with a nonsensical pointer for CurrentMemoryContext and crashing.


To build the C function DLL correctly it correctly do not define the
BUILDING_DLL macro. Instead, define your own DLL export macros like:

#if defined(_MSC_VER) || defined(__MINGW32__)
#define COPYTEXT_EXPORT __declspec (dllexport)
#else
#define COPYTEXT_EXPORT
#endif

then declare your function as:


COPYTEXT_EXPORT Datum copytext2(PG_FUNCTION_ARGS) {
     // blah blah
}


It should now be exported in the DLL's interface correctly AND have
correct access to Pg's exported functions and variables.

I did notice that I'm getting some warnings about inconsistent DLL
linkage for Pg_magic_func and pg_finfo_copytext . These really need to
be using __declspec(dllexport) rather than using __declspec(dllimport)
via PGDLLIMPORT .


Maybe it's worth providing a PGMODULEEXPORT macro for PG_MODULE_MAGIC,
PG_FUNCTION_INFO_V1, and for module writers to use to export their
functions in the DLL interface. A module author would have to ensure
that BUILDING_MODULE was defined.

Here's how the PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1 macros would look:

/* This might want to go somewhere other than fmgr.h, like
  * pg_config_os.h alongside the definition of PGDLLIMPORT
  */
#if defined(_MSC_VER) || defined(__MINGW32__)
   #if defined(BUILDING_MODULE)
     #define PGMODULEEXPORT __declspec (dllexport)
   #else
     // Never actually used
     #define PGMODULEEXPORT __declspec (dllimport)
   #endif
#else
   #define PGMODULEEXPORT
#endif

#define PG_MODULE_MAGIC \
PGMODULEEXPORT  Pg_magic_struct * \
PG_MAGIC_FUNCTION_NAME(void) \
{ \
    static const Pg_magic_struct Pg_magic_data = PG_MODULE_MAGIC_DATA; \
    return &Pg_magic_data; \
} \
extern int no_such_variable

#define PG_FUNCTION_INFO_V1(funcname) \
PGMODULEEXPORT const Pg_finfo_record * \
CppConcat(pg_finfo_,funcname) (void) \
{ \
    static const Pg_finfo_record my_finfo = { 1 }; \
    return &my_finfo; \
} \
extern int no_such_variable



I've tested this definition and it produces a DLL that links correctly
and does so without the warnings of inconsistent DLL linkage produced by
the original versions (which declared the function __declspec(dllimport)
then defined it).

Anyway, the C function examples need some changes to work correctly on
win32. I've attached updated versions. These redefine the
PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1 macros, but if you omit that
code the module will still built, just with warnings, and will still work.

--
Craig Ringer
/*
 * PostgreSQL example C functions.
 *
 * This file must be built as a shared library or dll and
 * placed into the PostgreSQL `lib' directory. On Windows
 * it must link to postgres.lib .
 *
 * postgresql/include/server must be on your header search path.
 * With MSVC++ on win32 so must postgresql/include/server/port/win32_msvc .
 * With MinGW use postgresql/include/server/port/win32 .
 */

#if defined(_MSC_VER) || defined(__MINGW32__)
  #ifndef _USE_32BIT_TIME_T
    #define _USE_32BIT_TIME_T
  #endif
#endif

/* BUILDING_DLL causes the declarations in Pg's headers to be declared
 * __declspec(dllexport) which will break DLL linkage. */
#ifdef BUILDING_DLL
  #error Do not define BUILDING_DLL when building extension libraries
#endif

/* Ensure that Pg_module_function and friends are declared __declspec(dllexport) */
#ifndef BUILDING_MODULE
  #define BUILDING_MODULE
#endif

#include "postgres.h"
#include <string.h>
#include "fmgr.h"
#include "utils/geo_decls.h"

/*--------------- BEGIN REDEFINITION OF PG MACROS -------------------
 *
 * These rewritten versions of PG_MODULE_MAGIC and PG_FUNCTION_INFO_V1
 * declare the module functions as __declspec(dllexport) when building
 * a module. They also provide PGMODULEEXPORT for exporting functions
 * in user DLLs.
 */
#undef PG_MODULE_MAGIC
#undef PG_FUNCTION_INFO_V1

/* This might want to go somewhere other than fmgr.h, like
 * pg_config_os.h alongside the definition of PGDLLIMPORT
 */
#if defined(_MSC_VER) || defined(__MINGW32__)
  #if defined(BUILDING_MODULE)
    #define PGMODULEEXPORT __declspec (dllexport)
  #else
    // Never actually used
    #define PGMODULEEXPORT __declspec (dllimport)
  #endif
#else
  #define PGMODULEEXPORT
#endif

#define PG_MODULE_MAGIC \
PGMODULEEXPORT  Pg_magic_struct * \
PG_MAGIC_FUNCTION_NAME(void) \
{ \
    static const Pg_magic_struct Pg_magic_data = PG_MODULE_MAGIC_DATA; \
    return &Pg_magic_data; \
} \
extern int no_such_variable

#define PG_FUNCTION_INFO_V1(funcname) \
PGMODULEEXPORT const Pg_finfo_record * \
CppConcat(pg_finfo_,funcname) (void) \
{ \
    static const Pg_finfo_record my_finfo = { 1 }; \
    return &my_finfo; \
} \
extern int no_such_variable

/*--------------- END REDEFINITION OF PG MACROS -------------------*/


PG_MODULE_MAGIC;

PG_FUNCTION_INFO_V1(add_one);

PGMODULEEXPORT Datum
add_one(PG_FUNCTION_ARGS)
{
    int32   arg = PG_GETARG_INT32(0);

    PG_RETURN_INT32(arg + 1);
}

/* by reference, fixed length */

PG_FUNCTION_INFO_V1(add_one_float8);

PGMODULEEXPORT Datum
add_one_float8(PG_FUNCTION_ARGS)
{
    /* The macros for FLOAT8 hide its pass-by-reference nature. */
    float8   arg = PG_GETARG_FLOAT8(0);

    PG_RETURN_FLOAT8(arg + 1.0);
}

PG_FUNCTION_INFO_V1(makepoint);

PGMODULEEXPORT Datum
makepoint(PG_FUNCTION_ARGS)
{
    /* Here, the pass-by-reference nature of Point is not hidden. */
    Point     *pointx = PG_GETARG_POINT_P(0);
    Point     *pointy = PG_GETARG_POINT_P(1);
    Point     *new_point = (Point *) palloc(sizeof(Point));

    new_point->x = pointx->x;
    new_point->y = pointy->y;

    PG_RETURN_POINT_P(new_point);
}

/* by reference, variable length */

PG_FUNCTION_INFO_V1(copytext);

PGMODULEEXPORT Datum
copytext(PG_FUNCTION_ARGS)
{
    text     *t = PG_GETARG_TEXT_P(0);
    /*
     * VARSIZE is the total size of the struct in bytes.
     */
    text     *new_t = (text *) palloc(VARSIZE(t));
    SET_VARSIZE(new_t, VARSIZE(t));
    /*
     * VARDATA is a pointer to the data region of the struct.
     */
    memcpy((void *) VARDATA(new_t), /* destination */
           (void *) VARDATA(t),     /* source */
           VARSIZE(t) - VARHDRSZ);  /* how many bytes */
    PG_RETURN_TEXT_P(new_t);
}

PG_FUNCTION_INFO_V1(concat_text);

PGMODULEEXPORT Datum
concat_text(PG_FUNCTION_ARGS)
{
    text  *arg1 = PG_GETARG_TEXT_P(0);
    text  *arg2 = PG_GETARG_TEXT_P(1);
    int32 new_text_size = VARSIZE(arg1) + VARSIZE(arg2) - VARHDRSZ;
    text *new_text = (text *) palloc(new_text_size);

    SET_VARSIZE(new_text, new_text_size);
    memcpy(VARDATA(new_text), VARDATA(arg1), VARSIZE(arg1) - VARHDRSZ);
    memcpy(VARDATA(new_text) + (VARSIZE(arg1) - VARHDRSZ),
           VARDATA(arg2), VARSIZE(arg2) - VARHDRSZ);
    PG_RETURN_TEXT_P(new_text);
}