Thread: Re: [GENERAL] Some questions on user defined types and functions.

Re: [GENERAL] Some questions on user defined types and functions.

From
Jeffery Collins
Date:
I moved the discussion from General to Hackers, as I am getting into actual
code changes now.  I hope this is appropriate.

Jeff


Thomas Lockhart wrote:

>
> There is code in the postmaster which does the same thing, nearly. You
> might want to check out the implementation there...
>
>                        - Thomas

I'm not exactly sure which code you are referring to.  I did see the
following places where environment variables in paths are expanded:

backend/utils/adt/filename.c:
    After I looked at this code for a while, I decided I didn't like it
enough to use.  My biggest concern with this code is that it only allows one
environment variable at the beginning of the path.  For example,
$PGHOME/rest/of/path is allowed, but $PGHOME/version_$VER/path is not
allowed.

backend/utils/misc/database.c:
    This code is only applicable to finding the path to the database
directory.  It has a hardwired 'base' in the expanded pathname.  Also only
environment variables in the beginning of the path are supported.

If y'all want the code, here are the diffs.  The are from 7.0.2.  I made,
what I think, are the appropriate changes to the documentation.  I do not
have a way to build the documentation so I can't see how my changes actually
look - I hope they are appropriately formatted.

Jeff

*** src/backend/commands/define.c.orig    Wed Jul 26 11:50:40 2000
--- src/backend/commands/define.c    Thu Jul 27 10:50:59 2000
***************
*** 182,194 ****
--- 182,204 ----
  interpret_AS_clause(const char *languageName, const List *as,
                      char **prosrc_str_p, char **probin_str_p)
  {
+ #if 0
      struct stat stat_buf;
+ #endif

      Assert(as != NIL);

      if (strcmp(languageName, "C") == 0)
      {

+ #if 0
+         /*
+          * It seems like you should be able to use a file that doesn't currently
+          * exist yet.  It only matters if the file exists when it is used.
+          * Also, if there is an environment variable in the pathname, doing the
+          * stat early might prevent the path from being inserted.
+          */
+
          /*
           * For "C" language, store the file name in probin and, when
           * given, the link symbol name in prosrc. But first, stat the
***************
*** 197,203 ****

          if (stat(strVal(lfirst(as)), &stat_buf) == -1)
                  elog(ERROR, "stat failed on file '%s': %m", strVal(lfirst(as)));
!
          *probin_str_p = strVal(lfirst(as));

          if (lnext(as) == NULL)
--- 207,213 ----

          if (stat(strVal(lfirst(as)), &stat_buf) == -1)
                  elog(ERROR, "stat failed on file '%s': %m", strVal(lfirst(as)));
! #endif
          *probin_str_p = strVal(lfirst(as));

          if (lnext(as) == NULL)
*** src/backend/utils/fmgr/dfmgr.c.orig    Wed Apr 12 13:15:57 2000
--- src/backend/utils/fmgr/dfmgr.c    Thu Jul 27 10:28:49 2000
***************
*** 36,41 ****
--- 36,42 ----
  static int    pronargs_save;
  static func_ptr user_fn_save = (func_ptr) NULL;
  static func_ptr handle_load(char *filename, char *funcname);
+ static int expand_filename(char *orig, char *new, int size);

  func_ptr
  fmgr_dynamic(Oid procedureId, int *pronargs)
***************
*** 51,56 ****
--- 52,59 ----
      func_ptr    user_fn;
      Relation    rel;
      bool        isnull;
+     char        filebuf[PATH_MAX];
+     char        *file_p;

      /* Implement simple one-element cache for function lookups */
      if (procedureId == procedureId_save)
***************
*** 120,126 ****

      heap_close(rel, AccessShareLock);

!     user_fn = handle_load(probinstring, linksymbol);

      pfree(probinstring);
      if (prosrcstring)
--- 123,138 ----

      heap_close(rel, AccessShareLock);

!     /* Expand any environment variables in the file name. */
!     if (strchr(probinstring, '$') != NULL) {
!         if (!expand_filename(probinstring, filebuf, sizeof(filebuf)))
!             elog(ERROR, "LOAD: could not expand file '%s': %m", probinstring);
!         file_p = filebuf;
!     }
!     else
!         file_p = probinstring;
!
!     user_fn = handle_load(file_p, linksymbol);

      pfree(probinstring);
      if (prosrcstring)
***************
*** 252,266 ****
  {
      DynamicFileList *file_scanner,
                 *p;
      struct stat stat_buf;
      int            done = 0;

      /*
       * We need to do stat() in order to determine whether this is the same
       * file as a previously loaded file; it's also handy so as to give a
       * good error message if bogus file name given.
       */
!     if (stat(filename, &stat_buf) == -1)
          elog(ERROR, "LOAD: could not open file '%s': %m", filename);

      if (file_list != (DynamicFileList *) NULL
--- 264,290 ----
  {
      DynamicFileList *file_scanner,
                 *p;
+     char        filebuf[PATH_MAX];
+     char        *file_p;
      struct stat stat_buf;
      int            done = 0;

+     /* Expand any environment variables in the file name. */
+     if (strchr(filename, '$') != NULL) {
+         if (!expand_filename(filename, filebuf, sizeof(filebuf)))
+             elog(ERROR, "LOAD: could not expand file '%s': %m", filename);
+         file_p = filebuf;
+         elog(NOTICE, "Expanded [%s] to [%s]", filename, file_p);
+     }
+     else
+         file_p = filename;
+
      /*
       * We need to do stat() in order to determine whether this is the same
       * file as a previously loaded file; it's also handy so as to give a
       * good error message if bogus file name given.
       */
!     if (stat(file_p, &stat_buf) == -1)
          elog(ERROR, "LOAD: could not open file '%s': %m", filename);

      if (file_list != (DynamicFileList *) NULL
***************
*** 292,298 ****
              free((char *) p);
          }
      }
!     handle_load(filename, (char *) NULL);
  }

  /* Is this used? bjm 1998/10/08   No. tgl 1999/02/07 */
--- 316,322 ----
              free((char *) p);
          }
      }
!     handle_load(file_p, (char *) NULL);
  }

  /* Is this used? bjm 1998/10/08   No. tgl 1999/02/07 */
***************
*** 308,310 ****
--- 332,411 ----
  }

  #endif
+
+ /*
+  * The filename has one or more environment variables.  Attempt to expand all
+  * of the environment variables to build an absolute path.
+  */
+
+ static int
+ expand_filename(char *orig, char *new, int size)
+ {
+     char    *copy;
+     char    *start;
+     char    *env_ptr;
+     char    env_var[MAXPGPATH];
+     char    *env_setting;
+     int        i;
+
+     /* Clear the 'new' pathname. */
+     *new = '\0';
+
+     /* Copy the string so it can be safely hacked */
+     copy = pstrdup(orig);
+
+     /* Walk the path while expanding the environment variables. */
+     start = copy;
+     while (strlen(start) > 0) {
+
+         /* Find the start of the next environment variable in the path. */
+         if ((env_ptr = strchr(start, '$')) != NULL) {
+             *env_ptr = '\0';
+
+             /* Append everything before the environment variable. */
+             if ((strlen(new) + strlen(start) + 1) > size) {
+                 pfree(copy);
+                 return(0);
+             }
+             strcat(new, start);
+
+             /* Copy the environment variable to its own string. */
+             env_ptr++;
+             i = 0;
+             while (*env_ptr != '\0' && *env_ptr != '/') {
+                 env_var[i] = *env_ptr;
+                 env_ptr++;
+                 i++;
+             }
+             env_var[i] = '\0';
+             start = env_ptr;
+
+             /* Translate the environment variable. */
+             if ((env_setting = getenv(env_var)) == NULL) {
+                 pfree(copy);
+                 return(0);
+             }
+
+             /* Add the environment variable to the path. */
+             if ((strlen(new) + strlen(env_setting) + 1) > size) {
+                 pfree(copy);
+                 return(0);
+             }
+             strcat(new, env_setting);
+         }
+
+         /*
+          * No environment variables left in the path.  So append the rest of
+          * the path and exit the loop.
+          */
+         else {
+             if ((strlen(new) + strlen(start) + 1) > size) {
+                 pfree(copy);
+                 return(0);
+             }
+             strcat(new, start);
+             break;
+         }
+     }
+     return(1);
+ }
*** doc/src/sgml/dfunc.sgml.orig    Thu Jul 27 10:32:36 2000
--- doc/src/sgml/dfunc.sgml    Thu Jul 27 10:40:25 2000
***************
*** 96,105 ****
     <itemizedlist>
      <listitem>
       <para>
!       Paths  given  to the create function command must be
        absolute paths (i.e., start with "/") that refer  to
        directories  visible  on  the  machine  on which the
        <productname>Postgres</productname> server is running.

        <tip>
         <para>
--- 96,113 ----
     <itemizedlist>
      <listitem>
       <para>
!       Paths  given  to the create function command must expand to
        absolute paths (i.e., start with "/") that refer  to
        directories  visible  on  the  machine  on which the
        <productname>Postgres</productname> server is running.
+       </para>
+      <para>
+       Environment variables in the path will be expanded by the
+       <productname>Postgres</productname> server.  The environment
+       variables are interpreted in the context of the server, so make
+       sure the server has the appropriate environment variables set
+       correctly.
+       </para>

        <tip>
         <para>
*** doc/src/sgml/ref/create_function.sgml.orig    Thu Jul 27 10:40:56 2000
--- doc/src/sgml/ref/create_function.sgml    Thu Jul 27 10:49:32 2000
***************
*** 117,123 ****
      containing the dynamically loadable object, and <replaceable
      class="parameter">link_symbol</replaceable>, is the object's link
      symbol which is the same as the name of the function in the C
!     language source code.
         </para>
        </listitem>
       </varlistentry>
--- 117,126 ----
      containing the dynamically loadable object, and <replaceable
      class="parameter">link_symbol</replaceable>, is the object's link
      symbol which is the same as the name of the function in the C
!     language source code. <productname>Postgres</productname> will
!     automatically expand environment variables in <replaceable
!     class="parameter">obj_file</replaceable>.  The expansion is done using
!     the <productname>Postgres</productname> server's environment.
         </para>
        </listitem>
       </varlistentry>

Re: Re: [GENERAL] Some questions on user defined types and functions.

From
Thomas Lockhart
Date:
> I moved the discussion from General to Hackers, as I am getting into actual
> code changes now.  I hope this is appropriate.

Yup.

> I'm not exactly sure which code you are referring to.  I did see the
> following places where environment variables in paths are expanded:
...
> backend/utils/misc/database.c:
>     This code is only applicable to finding the path to the database
> directory.  It has a hardwired 'base' in the expanded pathname.  Also only
> environment variables in the beginning of the path are supported.

This is the one I was thinking of. The "leading envar" is pretty
unambiguous; allowing them farther into the string will restrict paths
from having a dollar sign (not terribly important, but it is an obscure
restriction). Also, and probably more important, by requiring that the
envar be in the first position it is a simple one-byte comparison to see
if any expansion *may* need to be done. So the performance is not
affected at all if no environment variable is used.

> If y'all want the code, here are the diffs.  The are from 7.0.2.  I made,
> what I think, are the appropriate changes to the documentation.  I do not
> have a way to build the documentation so I can't see how my changes actually
> look - I hope they are appropriately formatted.

Thanks. Shall we tweak it to support the same conventions as for the
other cases (leading envar only)? It will remove any possible objection
regarding efficiency, and it will conform to the other usages (btw, the
"hardwired 'base/' in the database.c example could/should be considered
a security feature since it requires a well-formed directory structure).
                        - Thomas


Re: Re: [GENERAL] Some questions on user defined types and functions.

From
Jeffery Collins
Date:
Thomas Lockhart wrote:

> > I'm not exactly sure which code you are referring to.  I did see the
> > following places where environment variables in paths are expanded:
> ...
> > backend/utils/misc/database.c:
> >     This code is only applicable to finding the path to the database
> > directory.  It has a hardwired 'base' in the expanded pathname.  Also only
> > environment variables in the beginning of the path are supported.
>
> This is the one I was thinking of. The "leading envar" is pretty
> unambiguous; allowing them farther into the string will restrict paths
> from having a dollar sign (not terribly important, but it is an obscure
> restriction). Also, and probably more important, by requiring that the
> envar be in the first position it is a simple one-byte comparison to see
> if any expansion *may* need to be done. So the performance is not
> affected at all if no environment variable is used.
>
> > If y'all want the code, here are the diffs.  The are from 7.0.2.  I made,
> > what I think, are the appropriate changes to the documentation.  I do not
> > have a way to build the documentation so I can't see how my changes actually
> > look - I hope they are appropriately formatted.
>
> Thanks. Shall we tweak it to support the same conventions as for the
> other cases (leading envar only)? It will remove any possible objection
> regarding efficiency, and it will conform to the other usages (btw, the
> "hardwired 'base/' in the database.c example could/should be considered
> a security feature since it requires a well-formed directory structure).
>
>                          - Thomas

Feel free to change so that the envar must be leading.  It is definitely easier
code with this restriction and definately faster.

If you want, I will make the change.  If the restriction is desired, I can make
the code in database.c and my code common.

Jeff




Re: Re: [GENERAL] Some questions on user defined types and functions.

From
Thomas Lockhart
Date:
> If you want, I will make the change.  If the restriction is desired, I can 
> make the code in database.c and my code common.

Ooh. That sounds like a great offer. You are welcome to do the work :)

So, we will end up with a routine which will take a string, check for a
leading environment variable, and expand the envar if necessary? And we
will use that in two places?

istm that the above will require some memory allocation at times. I
haven't checked to see if the memory context or states for both cases
are compatible. If they aren't, then feel free to bring along two sets
of code.

Regards.
                    -Thomas