Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite60cb3a

Browse files
committed
Code review for magic-block patch. Remove separate header file pgmagic.h,
as this seems only likely to create headaches for module developers. Putthe macro in the pre-existing fmgr.h file instead. Avoid being too cuteabout how many fields we can cram into a word, and avoid trying to fetchfrom a library we've already unlinked.Along the way, it occurred to me that the magic block really ought to be'const' so it can be stored in the program text area. Do the same forthe existing data blocks for PG_FUNCTION_INFO_V1 functions.
1 parenta18ebc5 commite60cb3a

File tree

6 files changed

+146
-150
lines changed

6 files changed

+146
-150
lines changed

‎doc/src/sgml/xfunc.sgml

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/xfunc.sgml,v 1.113 2006/05/3014:09:32 momjian Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/xfunc.sgml,v 1.114 2006/05/3021:21:29 tgl Exp $ -->
22

33
<sect1 id="xfunc">
44
<title>User-Defined Functions</title>
@@ -1148,13 +1148,6 @@ CREATE FUNCTION square_root(double precision) RETURNS double precision
11481148
that fails as well, the load will fail.
11491149
</para>
11501150

1151-
<para>
1152-
After the module has been found, PostgreSQL looks for its magic block.
1153-
This block contains information about the environment a module was
1154-
compiled in. The server uses this to verify the module was compiled
1155-
under the same assumptions and environment as the backend.
1156-
</para>
1157-
11581151
<para>
11591152
The user ID the <productname>PostgreSQL</productname> server runs
11601153
as must be able to traverse the path to the file you intend to
@@ -1960,31 +1953,36 @@ concat_text(PG_FUNCTION_ARGS)
19601953

19611954
<listitem>
19621955
<para>
1963-
To ensure your module is not loaded into an incompatible backend, it
1964-
is recommended to include a magic block. To do this you must include
1965-
the header <filename>pgmagic.h</filename> and declare the block as
1966-
follows:
1956+
Symbol names defined within object files must not conflict
1957+
with each other or with symbols defined in the
1958+
<productname>PostgreSQL</productname> server executable. You
1959+
will have to rename your functions or variables if you get
1960+
error messages to this effect.
19671961
</para>
1962+
</listitem>
19681963

1969-
<programlisting>
1970-
#include "pgmagic.h"
1964+
<listitem>
1965+
<para>
1966+
To ensure your module is not loaded into an incompatible server, it
1967+
is recommended to include a <quote>magic block</>. This allows
1968+
the server to detect obvious incompatibilities, such as a module
1969+
compiled for a different major version of
1970+
<productname>PostgreSQL</productname>. It is likely that magic
1971+
blocks will be required in future releases. To include a magic
1972+
block, write this in one (and only one) of your module source files,
1973+
after having included the header <filename>fmgr.h</>:
1974+
</para>
19711975

1976+
<programlisting>
1977+
#ifdef PG_MODULE_MAGIC
19721978
PG_MODULE_MAGIC;
1979+
#endif
19731980
</programlisting>
19741981

19751982
<para>
1976-
If the module consists of multiple source files, this only needs to
1977-
be done in one of them.
1978-
</para>
1979-
</listitem>
1980-
1981-
<listitem>
1982-
<para>
1983-
Symbol names defined within object files must not conflict
1984-
with each other or with symbols defined in the
1985-
<productname>PostgreSQL</productname> server executable. You
1986-
will have to rename your functions or variables if you get
1987-
error messages to this effect.
1983+
The <literal>#ifdef</> test can be omitted if your code doesn't
1984+
need to compile against pre-8.2 <productname>PostgreSQL</productname>
1985+
releases.
19881986
</para>
19891987
</listitem>
19901988

‎src/backend/utils/fmgr/dfmgr.c

Lines changed: 45 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,18 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.83 2006/05/3014:09:32 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/fmgr/dfmgr.c,v 1.84 2006/05/3021:21:30 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
1515
#include"postgres.h"
1616

17-
#include<errno.h>
1817
#include<sys/stat.h>
1918

2019
#include"dynloader.h"
2120
#include"miscadmin.h"
2221
#include"utils/dynamic_loader.h"
23-
#include"pgmagic.h"
22+
2423

2524
/*
2625
* List of dynamically loaded files (kept in malloc'd memory).
@@ -61,7 +60,8 @@ static char *expand_dynamic_library_name(const char *name);
6160
staticchar*substitute_libpath_macro(constchar*name);
6261

6362
/* Magic structure that module needs to match to be accepted */
64-
staticPg_magic_structmagic_data=PG_MODULE_MAGIC_DATA;
63+
staticconstPg_magic_structmagic_data=PG_MODULE_MAGIC_DATA;
64+
6565

6666
/*
6767
* Load the specified dynamic-link library file, and look for a function
@@ -82,6 +82,7 @@ load_external_function(char *filename, char *funcname,
8282
{
8383
DynamicFileList*file_scanner;
8484
PGFunctionretval;
85+
PGModuleMagicFunctionmagic_func;
8586
char*load_error;
8687
structstatstat_buf;
8788
char*fullname;
@@ -119,7 +120,6 @@ load_external_function(char *filename, char *funcname,
119120

120121
if (file_scanner==NULL)
121122
{
122-
PGModuleMagicFunctionmagic_func;
123123
/*
124124
* File not loaded yet.
125125
*/
@@ -150,44 +150,53 @@ load_external_function(char *filename, char *funcname,
150150
fullname,load_error)));
151151
}
152152

153-
/* Check the magic function to determine compatability */
154-
magic_func=pg_dlsym(file_scanner->handle,PG_MAGIC_FUNCTION_NAME_STRING );
155-
if(magic_func )
153+
/* Check the magic function to determine compatibility */
154+
magic_func= (PGModuleMagicFunction)
155+
pg_dlsym(file_scanner->handle,PG_MAGIC_FUNCTION_NAME_STRING);
156+
if (magic_func)
156157
{
157-
Pg_magic_struct*module_magic_data=magic_func();
158-
if(module_magic_data->len!=magic_data.len||
159-
memcmp(module_magic_data,&magic_data,magic_data.len )!=0 )
158+
constPg_magic_struct*magic_data_ptr= (*magic_func) ();
159+
160+
if (magic_data_ptr->len!=magic_data.len||
161+
memcmp(magic_data_ptr,&magic_data,magic_data.len)!=0)
160162
{
161-
pg_dlclose(file_scanner->handle );
162-
163-
if(module_magic_data->len!=magic_data.len )
163+
/* copy data block before unlinking library */
164+
Pg_magic_structmodule_magic_data=*magic_data_ptr;
165+
166+
/* try to unlink library */
167+
pg_dlclose(file_scanner->handle);
168+
free((char*)file_scanner);
169+
170+
/*
171+
* Report suitable error. It's probably not worth writing
172+
* a separate error message for each field; only the most
173+
* common case of wrong major version gets its own message.
174+
*/
175+
if (module_magic_data.version!=magic_data.version)
164176
ereport(ERROR,
165-
(errmsg("incompatible library \"%s\": Magic block length mismatch",
166-
fullname)));
167-
if(module_magic_data->version!=magic_data.version )
168-
ereport(ERROR,
169-
(errmsg("incompatible library \"%s\": Version mismatch",
170-
fullname),
171-
errdetail("Expected %d.%d, got %d.%d",
172-
magic_data.version/100,magic_data.version %100,
173-
module_magic_data->version/100,module_magic_data->version %100)));
174-
175-
if(module_magic_data->magic!=magic_data.magic )
176-
ereport(ERROR,
177-
(errmsg("incompatible library \"%s\": Magic constant mismatch",
178-
fullname),
179-
errdetail("Expected 0x%08X, got 0x%08X",
180-
magic_data.magic,magic_data.magic)));
181-
/* Should never get here */
182-
ereport(ERROR,(errmsg("incompatible library \"%s\": Reason unknown",
177+
(errmsg("incompatible library \"%s\": version mismatch",
178+
fullname),
179+
errdetail("Server is version %d.%d, library is version %d.%d.",
180+
magic_data.version/100,
181+
magic_data.version %100,
182+
module_magic_data.version/100,
183+
module_magic_data.version %100)));
184+
ereport(ERROR,
185+
(errmsg("incompatible library \"%s\": magic block mismatch",
183186
fullname)));
184187
}
185188
}
186189
else
187-
/* Currently we do not penalize modules for not having a
188-
magic block, it would break every external module in
189-
existance. At some point though... */
190-
ereport(LOG, (errmsg("external library \"%s\" did not have magic block",fullname )));
190+
{
191+
/*
192+
* Currently we do not reject modules for not having a
193+
* magic block, it would break every external module in
194+
* existence. At some point though, this will become an ERROR.
195+
*/
196+
ereport(LOG,
197+
(errmsg("library \"%s\" does not have a magic block",
198+
fullname)));
199+
}
191200

192201
/* OK to link it into list */
193202
if (file_list==NULL)

‎src/backend/utils/fmgr/fmgr.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.100 2006/04/04 19:35:36 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/fmgr/fmgr.c,v 1.101 2006/05/30 21:21:30 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -66,7 +66,7 @@ typedef struct
6666
TransactionIdfn_xmin;/* for checking up-to-dateness */
6767
CommandIdfn_cmin;
6868
PGFunctionuser_fn;/* the function's address */
69-
Pg_finfo_record*inforec;/* address of its info record */
69+
constPg_finfo_record*inforec;/* address of its info record */
7070
}CFuncHashTabEntry;
7171

7272
staticHTAB*CFuncHash=NULL;
@@ -78,7 +78,7 @@ static void fmgr_info_C_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedur
7878
staticvoidfmgr_info_other_lang(OidfunctionId,FmgrInfo*finfo,HeapTupleprocedureTuple);
7979
staticCFuncHashTabEntry*lookup_C_func(HeapTupleprocedureTuple);
8080
staticvoidrecord_C_func(HeapTupleprocedureTuple,
81-
PGFunctionuser_fn,Pg_finfo_record*inforec);
81+
PGFunctionuser_fn,constPg_finfo_record*inforec);
8282
staticDatumfmgr_oldstyle(PG_FUNCTION_ARGS);
8383
staticDatumfmgr_security_definer(PG_FUNCTION_ARGS);
8484

@@ -276,7 +276,7 @@ fmgr_info_C_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple)
276276
Form_pg_procprocedureStruct= (Form_pg_proc)GETSTRUCT(procedureTuple);
277277
CFuncHashTabEntry*hashentry;
278278
PGFunctionuser_fn;
279-
Pg_finfo_record*inforec;
279+
constPg_finfo_record*inforec;
280280
Oldstyle_fnextra*fnextra;
281281
boolisnull;
282282
inti;
@@ -405,12 +405,12 @@ fmgr_info_other_lang(Oid functionId, FmgrInfo *finfo, HeapTuple procedureTuple)
405405
* can validate the information record for a function not yet entered into
406406
* pg_proc.
407407
*/
408-
Pg_finfo_record*
408+
constPg_finfo_record*
409409
fetch_finfo_record(void*filehandle,char*funcname)
410410
{
411411
char*infofuncname;
412412
PGFInfoFunctioninfofunc;
413-
Pg_finfo_record*inforec;
413+
constPg_finfo_record*inforec;
414414
staticPg_finfo_recorddefault_inforec= {0};
415415

416416
/* Compute name of info func */
@@ -493,7 +493,7 @@ lookup_C_func(HeapTuple procedureTuple)
493493
*/
494494
staticvoid
495495
record_C_func(HeapTupleprocedureTuple,
496-
PGFunctionuser_fn,Pg_finfo_record*inforec)
496+
PGFunctionuser_fn,constPg_finfo_record*inforec)
497497
{
498498
Oidfn_oid=HeapTupleGetOid(procedureTuple);
499499
CFuncHashTabEntry*entry;

‎src/include/fmgr.h

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
* Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group
1212
* Portions Copyright (c) 1994, Regents of the University of California
1313
*
14-
* $PostgreSQL: pgsql/src/include/fmgr.h,v 1.43 2006/04/04 19:35:37 tgl Exp $
14+
* $PostgreSQL: pgsql/src/include/fmgr.h,v 1.44 2006/05/30 21:21:30 tgl Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -293,24 +293,83 @@ typedef struct
293293
}Pg_finfo_record;
294294

295295
/* Expected signature of an info function */
296-
typedefPg_finfo_record*(*PGFInfoFunction) (void);
296+
typedefconstPg_finfo_record*(*PGFInfoFunction) (void);
297297

298298
/*
299299
*Macro to build an info function associated with the given function name.
300300
*Win32 loadable functions usually link with 'dlltool --export-all', but it
301301
*doesn't hurt to add DLLIMPORT in case they don't.
302302
*/
303303
#definePG_FUNCTION_INFO_V1(funcname) \
304-
extern DLLIMPORT Pg_finfo_record * CppConcat(pg_finfo_,funcname)(void); \
305-
Pg_finfo_record * \
304+
extern DLLIMPORTconstPg_finfo_record * CppConcat(pg_finfo_,funcname)(void); \
305+
constPg_finfo_record * \
306306
CppConcat(pg_finfo_,funcname) (void) \
307307
{ \
308-
static Pg_finfo_record my_finfo = { 1 }; \
308+
staticconstPg_finfo_record my_finfo = { 1 }; \
309309
return &my_finfo; \
310310
} \
311311
extern int no_such_variable
312312

313313

314+
/*-------------------------------------------------------------------------
315+
*Support for verifying backend compatibility of loaded modules
316+
*
317+
* If a loaded module includes the macro call
318+
*PG_MODULE_MAGIC;
319+
* (put this in only one source file), then we can check for obvious
320+
* incompatibility, such as being compiled for a different major PostgreSQL
321+
* version.
322+
*
323+
* To compile with versions of PostgreSQL that do not support this,
324+
* you may put an #ifdef/#endif test around it.
325+
*
326+
* The specific items included in the magic block are intended to be ones that
327+
* are custom-configurable and especially likely to break dynamically loaded
328+
* modules if they were compiled with other values. Also, the length field
329+
* can be used to detect definition changes.
330+
*-------------------------------------------------------------------------
331+
*/
332+
333+
/* Definition of the magic block structure */
334+
typedefstruct
335+
{
336+
intlen;/* sizeof(this struct) */
337+
intversion;/* PostgreSQL major version */
338+
intfuncmaxargs;/* FUNC_MAX_ARGS */
339+
intindexmaxkeys;/* INDEX_MAX_KEYS */
340+
intnamedatalen;/* NAMEDATALEN */
341+
}Pg_magic_struct;
342+
343+
/* The actual data block contents */
344+
#definePG_MODULE_MAGIC_DATA \
345+
{ \
346+
sizeof(Pg_magic_struct), \
347+
PG_VERSION_NUM / 100, \
348+
FUNC_MAX_ARGS, \
349+
INDEX_MAX_KEYS, \
350+
NAMEDATALEN \
351+
}
352+
353+
/*
354+
* Declare the module magic function. It needs to be a function as the dlsym
355+
* in the backend is only guaranteed to work on functions, not data
356+
*/
357+
typedefconstPg_magic_struct*(*PGModuleMagicFunction) (void);
358+
359+
#definePG_MAGIC_FUNCTION_NAME Pg_magic_func
360+
#definePG_MAGIC_FUNCTION_NAME_STRING "Pg_magic_func"
361+
362+
#definePG_MODULE_MAGIC \
363+
extern DLLIMPORT const Pg_magic_struct *PG_MAGIC_FUNCTION_NAME(void); \
364+
const Pg_magic_struct * \
365+
PG_MAGIC_FUNCTION_NAME(void) \
366+
{ \
367+
static const Pg_magic_struct Pg_magic_data = PG_MODULE_MAGIC_DATA; \
368+
return &Pg_magic_data; \
369+
} \
370+
extern int no_such_variable
371+
372+
314373
/*-------------------------------------------------------------------------
315374
*Support routines and macros for callers of fmgr-compatible functions
316375
*-------------------------------------------------------------------------
@@ -414,7 +473,7 @@ extern bytea *OidSendFunctionCall(Oid functionId, Datum val);
414473
/*
415474
* Routines in fmgr.c
416475
*/
417-
externPg_finfo_record*fetch_finfo_record(void*filehandle,char*funcname);
476+
externconstPg_finfo_record*fetch_finfo_record(void*filehandle,char*funcname);
418477
externvoidclear_external_function_hash(void*filehandle);
419478
externOidfmgr_internal_function(constchar*proname);
420479
externOidget_fn_expr_rettype(FmgrInfo*flinfo);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp