Re: [PATCHES] [PATCH] Round 2: Magic block for modules

2006-05-30 Thread Bruce Momjian

Patch applied.  Thanks.

---


Martijn van Oosterhout wrote:
-- Start of PGP signed section.
 Per feedback, here is an updated version. As was pointed out, the prior
 version was checking stuff that either changed too often to be useful
 or was never going to change at all. The error reporting is cleaned up
 too, it now releases the module before throwing the error. It now only
 checks four things:
 
 Major version number (7.4 or 8.1 for example)
 NAMEDATALEN
 FUNC_MAX_ARGS
 INDEX_MAX_KEYS
 
 The three constants were chosen because:
 
 1. We document them in the config page in the docs
 2. We mark them as changable in pg_config_manual.h
 3. Changing any of these will break some of the more popular modules:
 
 FUNC_MAX_ARGS changes fmgr interface, every module uses this
 NAMEDATALEN changes syscache interface, every PL as well as tsearch uses this
 INDEX_MAX_KEYS breaks tsearch and anything using GiST.
 
 I considered others but ultimatly rejected them. For example,
 HAVE_INT64_TIMESTAMP, while changing the way timestamps are stored and
 being configurable by a configure option, doesn't actually break
 anything important (only the btree_gist example in contrib).
 
 Any more comments?
 
 Have a nice day,
 -- 
 Martijn van Oosterhout   kleptog@svana.org   http://svana.org/kleptog/
  From each according to his ability. To each according to his ability to 
  litigate.

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

-- 
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDBhttp://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

---(end of broadcast)---
TIP 6: explain analyze is your friend


Re: [PATCHES] [PATCH] Round 2: Magic block for modules

2006-05-30 Thread Tom Lane
Bruce Momjian pgman@candle.pha.pa.us writes:
 Patch applied.  Thanks.

I hadn't gotten around to reviewing the revised version.  Just to let
you know, I'm going to remove the separate header file pgmagic.h and
put the macro into fmgr.h as I'd suggested originally.  The reason is
that the separate file turns the problem of making backward-compatible
modules from a simple #ifdef PG_MAGIC_BLOCK into a big does-that-
header-exist autoconf pushup.  It's not worth that.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


Re: [PATCHES] [PATCH] Round 2: Magic block for modules

2006-05-30 Thread Andrew Dunstan

Tom Lane wrote:

Bruce Momjian pgman@candle.pha.pa.us writes:
  

Patch applied.  Thanks.



I hadn't gotten around to reviewing the revised version. 


Is it just me or is this happening a lot lately?

cheers

andrew

---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
  subscribe-nomail command to [EMAIL PROTECTED] so that your
  message can get through to the mailing list cleanly


Re: [PATCHES] [PATCH] Round 2: Magic block for modules

2006-05-30 Thread Tom Lane
Andrew Dunstan [EMAIL PROTECTED] writes:
 Tom Lane wrote:
 I hadn't gotten around to reviewing the revised version. 

 Is it just me or is this happening a lot lately?

That security stuff took up a *lot* of time behind the scenes :-(

Normality is returning, slowly.

regards, tom lane

---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster


[PATCHES] [PATCH] Round 2: Magic block for modules

2006-05-08 Thread Martijn van Oosterhout
Per feedback, here is an updated version. As was pointed out, the prior
version was checking stuff that either changed too often to be useful
or was never going to change at all. The error reporting is cleaned up
too, it now releases the module before throwing the error. It now only
checks four things:

Major version number (7.4 or 8.1 for example)
NAMEDATALEN
FUNC_MAX_ARGS
INDEX_MAX_KEYS

The three constants were chosen because:

1. We document them in the config page in the docs
2. We mark them as changable in pg_config_manual.h
3. Changing any of these will break some of the more popular modules:

FUNC_MAX_ARGS changes fmgr interface, every module uses this
NAMEDATALEN changes syscache interface, every PL as well as tsearch uses this
INDEX_MAX_KEYS breaks tsearch and anything using GiST.

I considered others but ultimatly rejected them. For example,
HAVE_INT64_TIMESTAMP, while changing the way timestamps are stored and
being configurable by a configure option, doesn't actually break
anything important (only the btree_gist example in contrib).

Any more comments?

Have a nice day,
-- 
Martijn van Oosterhout   kleptog@svana.org   http://svana.org/kleptog/
 From each according to his ability. To each according to his ability to 
 litigate.
Index: doc/src/sgml/xfunc.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/xfunc.sgml,v
retrieving revision 1.112
diff -c -r1.112 xfunc.sgml
*** doc/src/sgml/xfunc.sgml 23 Apr 2006 03:39:52 -  1.112
--- doc/src/sgml/xfunc.sgml 8 May 2006 17:41:33 -
***
*** 1149,1154 
--- 1149,1161 
 /para
  
 para
+ After the module has been found, PostgreSQL looks for its magic block.
+ This block contains information about the environment a module was
+ compiled in. The server uses this to verify the module was compiled
+ under the same assumptions and environment as the backend.
+/para
+ 
+para
  The user ID the productnamePostgreSQL/productname server runs
  as must be able to traverse the path to the file you intend to
  load.  Making the file or a higher-level directory not readable
***
*** 1953,1958 
--- 1960,1985 
  
listitem
 para
+ To ensure your module is not loaded into an incompatible backend, it
+ is recommended to include a magic block. To do this you must include
+ the header filenamepgmagic.h/filename and declare the block as
+ follows:
+/para
+ 
+ programlisting
+ #include pgmagic.h
+ 
+ PG_MODULE_MAGIC;
+ /programlisting
+ 
+para
+ If the module consists of multiple source files, this only needs to
+ be done in one of them.
+/para
+   /listitem
+ 
+   listitem
+para
  Symbol names defined within object files must not conflict
  with each other or with symbols defined in the
  productnamePostgreSQL/productname server executable.  You
Index: src/backend/utils/fmgr/dfmgr.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/utils/fmgr/dfmgr.c,v
retrieving revision 1.82
diff -c -r1.82 dfmgr.c
*** src/backend/utils/fmgr/dfmgr.c  5 Mar 2006 15:58:46 -   1.82
--- src/backend/utils/fmgr/dfmgr.c  8 May 2006 17:41:33 -
***
*** 20,26 
  #include dynloader.h
  #include miscadmin.h
  #include utils/dynamic_loader.h
! 
  
  /*
   * List of dynamically loaded files (kept in malloc'd memory).
--- 20,26 
  #include dynloader.h
  #include miscadmin.h
  #include utils/dynamic_loader.h
! #include pgmagic.h
  
  /*
   * List of dynamically loaded files (kept in malloc'd memory).
***
*** 60,65 
--- 60,68 
  static char *expand_dynamic_library_name(const char *name);
  static char *substitute_libpath_macro(const char *name);
  
+ /* Magic structure that module needs to match to be accepted */
+ static Pg_magic_struct magic_data = PG_MODULE_MAGIC_DATA;
+ 
  /*
   * Load the specified dynamic-link library file, and look for a function
   * named funcname in it.  (funcname can be NULL to just load the file.)
***
*** 116,121 
--- 119,125 
  
if (file_scanner == NULL)
{
+   PGModuleMagicFunction magic_func;
/*
 * File not loaded yet.
 */
***
*** 146,151 
--- 150,194 
fullname, load_error)));
}
  
+   /* Check the magic function to determine compatability */
+   magic_func = pg_dlsym( file_scanner-handle, 
PG_MAGIC_FUNCTION_NAME_STRING );
+   if( magic_func )
+   {
+   Pg_magic_struct *module_magic_data = magic_func();
+   if( module_magic_data-len != magic_data.len ||
+   memcmp(