Thread: patch for src/backend/main/main.c

patch for src/backend/main/main.c

From
"Michael C. Thornburgh"
Date:

attached is a patch which fixes a bug related to
the use of getpwuid when running in standalone mode.
this patch allocates some persistent storage (using
malloc) to store the username obtained with getpwuid
in src/backend/main/main.c.  this is necessary because
later on, getpwuid is called again (in ValidateBinary).

the man pages for getpwuid on SCO OpenServer, FreeBSD,
and Darwin all have words to this effect (this is from
the SCO OpenServer man page):

  Note
  ====
  All information is contained in a static area, so it must
  be copied if it is to be saved. Otherwise, it may be
  overwritten on subsequent calls to these routines.

in particular, on my platform, the storage used to hold
the pw_name from the first call is overwritten such that
it looks like an empty username.  this causes a problem
later on in SetSessionUserIdFromUserName.

i'd assume this isn't a problem on most platforms because
getpwuid is called with the same UID both times, and the
same thing ends up happening to that static storage each
time.  however, that's not guaranteed, and is _not_ what
happens on my platform (at least :).

this is for the version of 7.1 available via anon cvs as
of Tue Jan 23 15:14:00 2001 PST:
  .../src/backend/main/main.c,v 1.37 2000/12/31 18:04:35 tgl Exp

-michael thornburgh, zenomt@armory.com



*** src/backend/main/main.c.orig    Mon Jan 22 17:09:50 2001
--- src/backend/main/main.c    Tue Jan 23 15:39:42 2001
***************
*** 53,58 ****
--- 53,59 ----
  {
      int            len;
      struct passwd *pw;
+     char * pw_name_persist;

      /*
       * Place platform-specific startup hacks here.  This is the right
***************
*** 158,163 ****
          fprintf(stderr, "%s: invalid current euid", argv[0]);
          exit(1);
      }

!     exit(PostgresMain(argc, argv, argc, argv, pw->pw_name));
  }
--- 159,172 ----
          fprintf(stderr, "%s: invalid current euid", argv[0]);
          exit(1);
      }
+     len = strlen(pw->pw_name);
+     pw_name_persist = (char *) malloc(len+1);
+     if (pw_name_persist == (char *)NULL)
+     {
+         fprintf(stderr, "%s: can't malloc for username\n", argv[0]);
+         exit(1);
+     }
+     strncpy(pw_name_persist, pw->pw_name, len+1);

!     exit(PostgresMain(argc, argv, argc, argv, pw_name_persist));
  }

Re: patch for src/backend/main/main.c

From
Tom Lane
Date:
"Michael C. Thornburgh" <zenomt@armory.com> writes:
> +     len = strlen(pw->pw_name);
> +     pw_name_persist = (char *) malloc(len+1);
> +     if (pw_name_persist == (char *)NULL)
> +     {
> +         fprintf(stderr, "%s: can't malloc for username\n", argv[0]);
> +         exit(1);
> +     }
> +     strncpy(pw_name_persist, pw->pw_name, len+1);

This could be simplified to
    pw_name_persist = strdup(pw->pw_name);
no?

            regards, tom lane

Re: patch for src/backend/main/main.c

From
"Michael C. Thornburgh"
Date:
that certainly works and is much cleaner, but strdup
may not be as ubiquitous as malloc & strncpy.
someone more versed in portability issues than i am
should speak to that.

-michael thornburgh


> From: Tom Lane <tgl@sss.pgh.pa.us>
>
> "Michael C. Thornburgh" <zenomt@armory.com> writes:
> > +     len = strlen(pw->pw_name);
> > +     pw_name_persist = (char *) malloc(len+1);
> > +     if (pw_name_persist == (char *)NULL)
> > +     {
> > +         fprintf(stderr, "%s: can't malloc for username\n", argv[0]);
> > +         exit(1);
> > +     }
> > +     strncpy(pw_name_persist, pw->pw_name, len+1);
>
> This could be simplified to
>     pw_name_persist = strdup(pw->pw_name);
> no?
>
>             regards, tom lane

Re: Re: patch for src/backend/main/main.c

From
Bruce Momjian
Date:
We already use strdup a lot.  Want to send a new patch?


>
> that certainly works and is much cleaner, but strdup
> may not be as ubiquitous as malloc & strncpy.
> someone more versed in portability issues than i am
> should speak to that.
>
> -michael thornburgh
>
>
> > From: Tom Lane <tgl@sss.pgh.pa.us>
> >
> > "Michael C. Thornburgh" <zenomt@armory.com> writes:
> > > +     len = strlen(pw->pw_name);
> > > +     pw_name_persist = (char *) malloc(len+1);
> > > +     if (pw_name_persist == (char *)NULL)
> > > +     {
> > > +         fprintf(stderr, "%s: can't malloc for username\n", argv[0]);
> > > +         exit(1);
> > > +     }
> > > +     strncpy(pw_name_persist, pw->pw_name, len+1);
> >
> > This could be simplified to
> >     pw_name_persist = strdup(pw->pw_name);
> > no?
> >
> >             regards, tom lane
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Re: patch for src/backend/main/main.c

From
Alfred Perlstein
Date:
* Michael C. Thornburgh <zenomt@armory.com> [010123 17:33] wrote:
>
> that certainly works and is much cleaner, but strdup
> may not be as ubiquitous as malloc & strncpy.
> someone more versed in portability issues than i am
> should speak to that.

hmm:

HISTORY
     The strdup() function first appeared in 4.4BSD.

however:

http://www.abisource.com/mailinglists/abiword-dev/99/March/0038.html

  "strdup is no in the ANSI standard"

however:

~/pgcvs % find . -name "*.c" | xargs grep strdup | grep -v pstrdup | wc -l
     563

  (pstrdup() seems to be some internal postgresql function.)

So I think anyone who wants to port should provide thier own
function.

--
-Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
"I have the heart of a child; I keep it in a jar on my desk."

Re: Re: patch for src/backend/main/main.ch

From
Bruce Momjian
Date:
See:

    ./utils/strdup.c

We use our own if configure does not find it.


> * Michael C. Thornburgh <zenomt@armory.com> [010123 17:33] wrote:
> >
> > that certainly works and is much cleaner, but strdup
> > may not be as ubiquitous as malloc & strncpy.
> > someone more versed in portability issues than i am
> > should speak to that.
>
> hmm:
>
> HISTORY
>      The strdup() function first appeared in 4.4BSD.
>
> however:
>
> http://www.abisource.com/mailinglists/abiword-dev/99/March/0038.html
>
>   "strdup is no in the ANSI standard"
>
> however:
>
> ~/pgcvs % find . -name "*.c" | xargs grep strdup | grep -v pstrdup | wc -l
>      563
>
>   (pstrdup() seems to be some internal postgresql function.)
>
> So I think anyone who wants to port should provide thier own
> function.
>
> --
> -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org]
> "I have the heart of a child; I keep it in a jar on my desk."
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Re: patch for src/backend/main/main.c

From
"Michael C. Thornburgh"
Date:
> We already use strdup a lot.  Want to send a new patch?



attached is take-2 of a patch which fixes a bug related
to the use of getpwuid when running in standalone mode.
this patch allocates some persistent storage (using
strdup) to store the username obtained with getpwuid
in src/backend/main/main.c.  this is necessary because
later on, getpwuid is called again (in ValidateBinary).

the man pages for getpwuid on SCO OpenServer, FreeBSD,
and Darwin all have words to this effect (this is from
the SCO OpenServer man page):

  Note
  ====
  All information is contained in a static area, so it must
  be copied if it is to be saved. Otherwise, it may be
  overwritten on subsequent calls to these routines.

in particular, on my platform, the storage used to hold
the pw_name from the first call is overwritten such that
it looks like an empty username.  this causes a problem
later on in SetSessionUserIdFromUserName.

i'd assume this isn't a problem on most platforms because
getpwuid is called with the same UID both times, and the
same thing ends up happening to that static storage each
time.  however, that's not guaranteed, and is _not_ what
happens on my platform (at least :).

this is for the version of 7.1 available via anon cvs as
of Tue Jan 23 15:14:00 2001 PST:
  .../src/backend/main/main.c,v 1.37 2000/12/31 18:04:35 tgl Exp

-michael thornburgh, zenomt@armory.com



*** src/backend/main/main.c.orig    Mon Jan 22 17:09:50 2001
--- src/backend/main/main.c    Tue Jan 23 17:00:07 2001
***************
*** 53,58 ****
--- 53,59 ----
  {
      int            len;
      struct passwd *pw;
+     char * pw_name_persist;

      /*
       * Place platform-specific startup hacks here.  This is the right
***************
*** 158,163 ****
          fprintf(stderr, "%s: invalid current euid", argv[0]);
          exit(1);
      }

!     exit(PostgresMain(argc, argv, argc, argv, pw->pw_name));
  }
--- 159,165 ----
          fprintf(stderr, "%s: invalid current euid", argv[0]);
          exit(1);
      }
+     pw_name_persist = strdup(pw->pw_name);

!     exit(PostgresMain(argc, argv, argc, argv, pw_name_persist));
  }

Re: Re: patch for src/backend/main/main.c

From
Bruce Momjian
Date:
Thanks.  Applied.

>
> > We already use strdup a lot.  Want to send a new patch?
>
>
>
> attached is take-2 of a patch which fixes a bug related
> to the use of getpwuid when running in standalone mode.
> this patch allocates some persistent storage (using
> strdup) to store the username obtained with getpwuid
> in src/backend/main/main.c.  this is necessary because
> later on, getpwuid is called again (in ValidateBinary).
>
> the man pages for getpwuid on SCO OpenServer, FreeBSD,
> and Darwin all have words to this effect (this is from
> the SCO OpenServer man page):
>
>   Note
>   ====
>   All information is contained in a static area, so it must
>   be copied if it is to be saved. Otherwise, it may be
>   overwritten on subsequent calls to these routines.
>
> in particular, on my platform, the storage used to hold
> the pw_name from the first call is overwritten such that
> it looks like an empty username.  this causes a problem
> later on in SetSessionUserIdFromUserName.
>
> i'd assume this isn't a problem on most platforms because
> getpwuid is called with the same UID both times, and the
> same thing ends up happening to that static storage each
> time.  however, that's not guaranteed, and is _not_ what
> happens on my platform (at least :).
>
> this is for the version of 7.1 available via anon cvs as
> of Tue Jan 23 15:14:00 2001 PST:
>   .../src/backend/main/main.c,v 1.37 2000/12/31 18:04:35 tgl Exp
>
> -michael thornburgh, zenomt@armory.com
>
>
>
> *** src/backend/main/main.c.orig    Mon Jan 22 17:09:50 2001
> --- src/backend/main/main.c    Tue Jan 23 17:00:07 2001
> ***************
> *** 53,58 ****
> --- 53,59 ----
>   {
>       int            len;
>       struct passwd *pw;
> +     char * pw_name_persist;
>
>       /*
>        * Place platform-specific startup hacks here.  This is the right
> ***************
> *** 158,163 ****
>           fprintf(stderr, "%s: invalid current euid", argv[0]);
>           exit(1);
>       }
>
> !     exit(PostgresMain(argc, argv, argc, argv, pw->pw_name));
>   }
> --- 159,165 ----
>           fprintf(stderr, "%s: invalid current euid", argv[0]);
>           exit(1);
>       }
> +     pw_name_persist = strdup(pw->pw_name);
>
> !     exit(PostgresMain(argc, argv, argc, argv, pw_name_persist));
>   }
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026

Re: Re: patch for src/backend/main/main.c

From
Tom Lane
Date:
"Michael C. Thornburgh" <zenomt@armory.com> writes:
>> We already use strdup a lot.  Want to send a new patch?

> attached is take-2 of a patch which fixes a bug related
> to the use of getpwuid when running in standalone mode.

I should have mentioned one more request, which is a comment
in the code explaining just *why* the strdup is needed.
Otherwise somebody will delete it someday.

Bruce already applied your v2 patch, but if you can offer
a one-or-two-line summary suitable for insertion as a comment,
it'd be appreciated.

            regards, tom lane

Re: Re: patch for src/backend/main/main.c

From
Bruce Momjian
Date:
Comment added.

> "Michael C. Thornburgh" <zenomt@armory.com> writes:
> >> We already use strdup a lot.  Want to send a new patch?
>
> > attached is take-2 of a patch which fixes a bug related
> > to the use of getpwuid when running in standalone mode.
>
> I should have mentioned one more request, which is a comment
> in the code explaining just *why* the strdup is needed.
> Otherwise somebody will delete it someday.
>
> Bruce already applied your v2 patch, but if you can offer
> a one-or-two-line summary suitable for insertion as a comment,
> it'd be appreciated.
>
>             regards, tom lane
>


--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
? config.log
? config.cache
? config.status
? GNUmakefile
? src/Makefile.custom
? src/GNUmakefile
? src/Makefile.global
? src/log
? src/crtags
? src/backend/port/Makefile
? src/include/config.h
? src/include/stamp-h
Index: src/backend/main/main.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/main/main.c,v
retrieving revision 1.38
diff -c -r1.38 main.c
*** src/backend/main/main.c    2001/01/24 03:50:06    1.38
--- src/backend/main/main.c    2001/01/24 05:22:55
***************
*** 159,164 ****
--- 159,165 ----
          fprintf(stderr, "%s: invalid current euid", argv[0]);
          exit(1);
      }
+     /* Allocate new memory because later getpwuid() calls can overwrite it */
      pw_name_persist = strdup(pw->pw_name);

      exit(PostgresMain(argc, argv, argc, argv, pw_name_persist));