Thread: pgsql: * Stephen Frost (sfrost@snowman.net) wrote: > I've now tested
pgsql: * Stephen Frost (sfrost@snowman.net) wrote: > I've now tested
From
momjian@postgresql.org (Bruce Momjian)
Date:
Log Message:
-----------
* Stephen Frost (sfrost@snowman.net) wrote:
> I've now tested this patch at home w/ 8.2HEAD and it seems to fix the
> bug. I plan on testing it under 8.1.2 at work tommorow with
> mod_auth_krb5, etc, and expect it'll work there. Assuming all goes
> well and unless someone objects I'll forward the patch to -patches.
> It'd be great to have this fixed as it'll allow us to use Kerberos to
> authenticate to phppgadmin and other web-based tools which use
> Postgres.
While playing with this patch under 8.1.2 at home I discovered a
mistake in how I manually applied one of the hunks to fe-auth.c.
Basically, the base code had changed and so the patch needed to be
modified slightly. This is because the code no longer either has a
freeable pointer under 'name' or has 'name' as NULL.
The attached patch correctly frees the string from pg_krb5_authname
(where it had been strdup'd) if and only if pg_krb5_authname returned
a string (as opposed to falling through and having name be set using
name = pw->name;). Also added a comment to this effect.
Backpatch to 8.1.X.
Stephen Frost
Tags:
----
REL8_1_STABLE
Modified Files:
--------------
pgsql/src/interfaces/libpq:
fe-auth.c (r1.107.2.1 -> r1.107.2.2)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/interfaces/libpq/fe-auth.c.diff?r1=1.107.2.1&r2=1.107.2.2)
momjian@postgresql.org (Bruce Momjian) writes:
> Log Message:
> -----------
> * Stephen Frost (sfrost@snowman.net) wrote:
>> I've now tested this patch at home w/ 8.2HEAD and it seems to fix the
>> bug. I plan on testing it under 8.1.2 at work tommorow with
>> mod_auth_krb5, etc, and expect it'll work there. Assuming all goes
>> well and unless someone objects I'll forward the patch to -patches.
>> It'd be great to have this fixed as it'll allow us to use Kerberos to
>> authenticate to phppgadmin and other web-based tools which use
>> Postgres.
> While playing with this patch under 8.1.2 at home I discovered a
> mistake in how I manually applied one of the hunks to fe-auth.c.
> Basically, the base code had changed and so the patch needed to be
> modified slightly. This is because the code no longer either has a
> freeable pointer under 'name' or has 'name' as NULL.
> The attached patch correctly frees the string from pg_krb5_authname
> (where it had been strdup'd) if and only if pg_krb5_authname returned
> a string (as opposed to falling through and having name be set using
> name = pw->name;). Also added a comment to this effect.
Bruce, people would appreciate it if you made some effort to ensure
that commit messages describe the main purposes of the patch, rather
than being just the verbatim text of the last message in the thread.
The above log message is not merely overly verbose, but completely
useless. One might guess that the change had something to do with
Kerberos, but what?
regards, tom lane
Tom Lane wrote: > momjian@postgresql.org (Bruce Momjian) writes: > > Log Message: > > ----------- > > * Stephen Frost (sfrost@snowman.net) wrote: > >> I've now tested this patch at home w/ 8.2HEAD and it seems to fix the > >> bug. I plan on testing it under 8.1.2 at work tommorow with > >> mod_auth_krb5, etc, and expect it'll work there. Assuming all goes > >> well and unless someone objects I'll forward the patch to -patches. > >> It'd be great to have this fixed as it'll allow us to use Kerberos to > >> authenticate to phppgadmin and other web-based tools which use > >> Postgres. > > > While playing with this patch under 8.1.2 at home I discovered a > > mistake in how I manually applied one of the hunks to fe-auth.c. > > Basically, the base code had changed and so the patch needed to be > > modified slightly. This is because the code no longer either has a > > freeable pointer under 'name' or has 'name' as NULL. > > > The attached patch correctly frees the string from pg_krb5_authname > > (where it had been strdup'd) if and only if pg_krb5_authname returned > > a string (as opposed to falling through and having name be set using > > name = pw->name;). Also added a comment to this effect. > > Bruce, people would appreciate it if you made some effort to ensure > that commit messages describe the main purposes of the patch, rather > than being just the verbatim text of the last message in the thread. > The above log message is not merely overly verbose, but completely > useless. One might guess that the change had something to do with > Kerberos, but what? First I am hearing a complaint. I will try to clean them up. -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. +
Bruce Momjian wrote: >>Bruce, people would appreciate it if you made some effort to ensure >>that commit messages describe the main purposes of the patch, rather >>than being just the verbatim text of the last message in the thread. >>The above log message is not merely overly verbose, but completely >>useless. One might guess that the change had something to do with >>Kerberos, but what? >> >> > >First I am hearing a complaint. I will try to clean them up. > > > Well, not everyone is as assertive as Tom :-) One other thing: it's important that the first words in your commit mesage summarise the contents of the commit, because that's what gets put in the subject line of the committers email message. Recent examples can be seen here: http://archives.postgresql.org/pgsql-committers/2006-03/index.php I don't read every committers message, and the subject lines can help me filter them intelligently. cheers andrew
OK. --------------------------------------------------------------------------- Andrew Dunstan wrote: > Bruce Momjian wrote: > > >>Bruce, people would appreciate it if you made some effort to ensure > >>that commit messages describe the main purposes of the patch, rather > >>than being just the verbatim text of the last message in the thread. > >>The above log message is not merely overly verbose, but completely > >>useless. One might guess that the change had something to do with > >>Kerberos, but what? > >> > >> > > > >First I am hearing a complaint. I will try to clean them up. > > > > > > > > Well, not everyone is as assertive as Tom :-) > > One other thing: it's important that the first words in your commit > mesage summarise the contents of the commit, because that's what gets > put in the subject line of the committers email message. Recent examples > can be seen here: > http://archives.postgresql.org/pgsql-committers/2006-03/index.php > > I don't read every committers message, and the subject lines can help me > filter them intelligently. > > cheers > > andrew > -- Bruce Momjian http://candle.pha.pa.us SRA OSS, Inc. http://www.sraoss.com + If your life is a hard drive, Christ can be your backup. +
Andrew Dunstan <andrew@dunslane.net> writes: > One other thing: it's important that the first words in your commit > mesage summarise the contents of the commit, because that's what gets > put in the subject line of the committers email message. One thing I've been a bit mystified by is the effect of punctuation on the email subjects. I do compose my commit messages with some thought to what the subject will look like, but I got burnt recently here: http://archives.postgresql.org/pgsql-committers/2006-03/msg00056.php where the subject got truncated at a period. It seems to sometimes truncate at punctuation and sometimes not --- there are plenty of other examples where periods don't end the subject. Does anyone know the exact rules the message-bot is using? regards, tom lane