bug-cvs
[Top][All Lists]
Advanced

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

Re: Compile error in current CVS, src/server.c:5500


From: Derek Robert Price
Subject: Re: Compile error in current CVS, src/server.c:5500
Date: Mon, 21 Jul 2003 15:39:36 -0400
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1

Brian Murphy wrote:

Derek Robert Price wrote:

What exactly is the "error 0" you cite? I'd rather CVS not start outputting arbitrary error numbers without good reason. Also, if pam_strerror is called on the output of pam_end, I don't think printing the actual error number matters much. It shouldn't matter to an end user and likely not even to us with a good text message. Then again, and please feel free to correct me on this one, maybe we should leave the error number in there through the experimental stage, until we discover that the text messages in error reports are satisfying?

From the linux source the only other code returned from pam_end other than PAM_SUCCESS is PAM_SYSTEM_ERR which gives the message "System error" which doesn't explain a lot.


Still, as long as the PAM folks left themselves a way to improve their error messages in their API, we should use it and give them the chance. Perhaps actually outputting the retval for the user to see is pointless, but I think I like it, at least until we're out of the experimental stage. Partly because I'm not very familiar with PAM, of course.

Here is a patch fixing the nesting problem.


I think you resent your previous patch. I added a comment I missed the first time through anyhow. Also, please don't forget your ChangeLog entry.

Index: src/server.c
===================================================================
RCS file: /cvs/ccvs/src/server.c,v
retrieving revision 1.301
diff -u -r1.301 server.c
--- src/server.c        20 Jul 2003 16:38:55 -0000      1.301
+++ src/server.c        21 Jul 2003 18:54:54 -0000
@@ -5483,7 +5483,7 @@
    char *username, *password;
{
    pam_handle_t *pamh = NULL;
-    int retval;
+    int retval, err;
    struct cvs_pam_userinfo ui = { username, password };
    struct pam_conv conv = { cvs_pam_conv, (void *)&ui };

@@ -5491,14 +5491,19 @@

    if (retval == PAM_SUCCESS)
        retval = pam_authenticate(pamh, 0);
+    else
+       printf("E PAM Error: %s\n", pam_strerror(NULL, retval));

    if (retval == PAM_SUCCESS)
        retval = pam_acct_mgmt(pamh, 0);
+    else
+       printf("E PAM Error: %s\n", pam_strerror(pamh, retval));

-    if (pam_end(pamh,retval) != PAM_SUCCESS)
+    if ((err = pam_end(pamh, retval)) != PAM_SUCCESS)


There's nothing wrong with retval = pam_end(pamh, retval), assuming you've already checked the previous retval and printed the relevant error messages. There's no need for `err' here.

    {
-       printf("E Fatal error, aborting.\n
-               pam failed to release authenticator\n");
+       printf("E Fatal error, aborting.\n\
+error 0 pam failed to release authenticator\n\
+PAM error number %d: %s\n", err, pam_strerror(NULL, err));
        error_exit ();
    }

Derek

--
               *8^)

Email: address@hidden

Get CVS support at <http://ximbiot.com>!
--
It is error alone which needs the support of government.  Truth can stand by 
itself.
                        - Thomas Jefferson






reply via email to

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