patch for src/backend/main/main.c

"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

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

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

* 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."

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

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));
}

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

"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

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