bug-cvs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Fwd: problems trying to use both PAM and CVSROOT/passwd]


From: Derek Price
Subject: Re: [Fwd: problems trying to use both PAM and CVSROOT/passwd]
Date: Wed, 19 Jan 2005 17:09:49 -0500
User-agent: Mozilla Thunderbird 1.0 (Windows/20041206)

I take back my tentative approval for this patch. It didn't even compile here with FC1 & the nature of the problems make me wonder that it compiled anywhere.

Regardless, I've attached a new version of the patch to the issue <https://ccvs.cvshome.org/issues/show_bug.cgi?id=230>. I fixed the logical problems, as well as tidying some of the style issues. It should do what I think Brian was attempting, but I really have almost exactly zero PAM experience and little inclination to learn without major funding, so someone else is going to need to test it.

I also commented the issue that Brian's patch at least leaves a little to be desired in the way of commenting. At the very least, it is strange that pam_initialize(), which is always called, needs username and password, and the later call to check_password(), which is not always called, does not. This may be unavoidable, but I think it at least deserves an explanation in the header comment block of pam_initialize().

Regards,

Derek

Derek Price wrote:

Brian Murphy wrote:

This is a bug. When CVSROOT/passwd is used for authentication then the PAM library is not initialized. When the pam_* functions are called in switch_to_user
they fail because pam_start has not been called.

Solution:

split out pam initialization to a seperate function which is always called.

patch attached.

Please review and give your OK to submit.



You're using a different indentation style than the one specified in HACKING (what I spotted was where you put the open bracket which starts a new block after an `if'). It looks like you already got away with the new style at least once, and I'm not going to be a stickler, but if you have time to fix it, it would be nice to remain consistent. Regardless, fixing the PAM support is more important than the indentation style.

Other than that, the patch looks fine to me. Assuming it compiles and passes regression, feel free to commit it.

Cheers,

Derek


_______________________________________________
Bug-cvs mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/bug-cvs






reply via email to

[Prev in Thread] Current Thread [Next in Thread]