Skip site navigation (1) Skip section navigation (2)

Re: PAM patch...

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: "Dominic J(dot) Eidson" <sauron(at)the-infinite(dot)org>,pgsql-patches(at)postgresql(dot)org
Subject: Re: PAM patch...
Date: 2002-02-22 05:00:51
Message-ID: 14205.1014354051@sss.pgh.pa.us (view raw or flat)
Thread:
Lists: pgsql-patches
Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> Not sure if this is 7.2.1 material because it is only an error message
> reporting problem.  Comments?

Looks like a bug fix to me.  Since it can't possibly break anything
except PAM, I'd rate it as low-risk; hence, reasonable to apply to 7.2.*
too, once we're happy with it.

However, I don't like the patch as given.  It seems to perpetuate rather
than replace the fundamentally bogus coding style that led to the
problem in the first place.  ISTM that we have a sequence of PAM calls
here that each might fail, and if so we want to report an appropriate
message and return STATUS_ERROR, rather than proceed with the next call.
But as coded, the error handling for call X appears after call X+1!
No wonder it was broken; who could understand this?

I think that the coding pattern shown in lines 746-760 is good:

	retval = pam_something(params);

	if (retval != PAM_SUCCESS)
	{
		generate error message;
		clean up state as needed;
		return STATUS_ERROR;
	}

and that the right fix is to make each of the subsequent calls be in
this same pattern, not to try to emulate their nonsensical style.

			regards, tom lane

In response to

Responses

pgsql-patches by date

Next:From: Dominic J. EidsonDate: 2002-02-22 05:04:44
Subject: Re: PAM patch...
Previous:From: Bruce MomjianDate: 2002-02-22 04:45:14
Subject: Re: PAM patch...

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group