Bruce Momjian <pgman@candle.pha.pa.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
