[PATCH:xdm 5/5] Simplify FailedLogin code

[walter harms]

mmmh, you are changing
FailedLogin (d, struct greet_info) into FailedLogin (d, char *)

maybe it is better to keep greet_info as future version
may need additional info. I would stick with struct greet_info
and use greet->name instead username.

It is not a real problem just my experience.

re,
 wh


Am 03.06.2011 07:10, schrieb Alan Coopersmith:
> Relies on username going out of scope to discard the pointer returned by
> pam_get_item that pam_end frees at the bottom of the loop.
> 
> Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
> ---
>  greeter/greet.c |   50 +++++++++++++++++++++++---------------------------
>  1 files changed, 23 insertions(+), 27 deletions(-)
> 
> diff --git a/greeter/greet.c b/greeter/greet.c
> index 8426a65..87d2a83 100644
> --- a/greeter/greet.c
> +++ b/greeter/greet.c
> @@ -402,11 +402,9 @@ Greet (struct display *d, struct greet_info *greet)
>  
>  
>  static void
> -FailedLogin (struct display *d, struct greet_info *greet)
> +FailedLogin (struct display *d, const char *username)
>  {
>  #ifdef USE_SYSLOG
> -    const char *username = greet->name;
> -
>      if (username == NULL)
>  	username = "username unavailable";
>  
> @@ -415,10 +413,6 @@ FailedLogin (struct display *d, struct greet_info *greet)
>  	   d->name, username);
>  #endif
>      DrawFail (login);
> -#ifndef USE_PAM
> -    bzero (greet->name, strlen(greet->name));
> -    bzero (greet->password, strlen(greet->password));
> -#endif
>  }
>  
>  _X_EXPORT
> @@ -500,7 +494,6 @@ greet_user_rtn GreetUser(
>  	struct myconv_data pcd		= { d, greet, NULL };
>  	struct pam_conv   pc 		= { pamconv, &pcd };
>  	const char *	  pam_fname;
> -	char *		  username	= NULL;
>  	const char *	  login_prompt;
>  
>  
> @@ -586,12 +579,16 @@ greet_user_rtn GreetUser(
>  
>  	RUN_AND_CHECK_PAM_ERROR(pam_setcred,
>  				(*pamhp, 0));
> -	RUN_AND_CHECK_PAM_ERROR(pam_get_item,
> -				(*pamhp, PAM_USER, (void *) &username));
> -	if (username != NULL) {
> -	    Debug("PAM_USER: %s\n", username);
> -	    greet->name = username;
> -	    greet->password = NULL;
> +	{
> +	    char *username	= NULL;
> +
> +	    RUN_AND_CHECK_PAM_ERROR(pam_get_item,
> +				    (*pamhp, PAM_USER, (void *) &username));
> +	    if (username != NULL) {
> +		Debug("PAM_USER: %s\n", username);
> +		greet->name = username;
> +		greet->password = NULL;
> +	    }
>  	}
>  
>        pam_done:
> @@ -606,19 +603,14 @@ greet_user_rtn GreetUser(
>  	    break;
>  	} else {
>  	    /* Try to fill in username for failed login error log */
> -	    if (greet->name == NULL) {
> -		if (username == NULL) {
> -		    RUN_AND_CHECK_PAM_ERROR(pam_get_item,
> -					    (*pamhp, PAM_USER,
> -					     (void *) &username));
> -		}
> -		greet->name = username;
> -	    }
> -	    FailedLogin (d, greet);
> -	    if (greet->name == username) {
> -		/* pam_end frees the value returned by pam_get_item */
> -		greet->name = NULL;
> +	    char *username = greet->name;
> +
> +	    if (username == NULL) {
> +		RUN_AND_CHECK_PAM_ERROR(pam_get_item,
> +					(*pamhp, PAM_USER,
> +					 (void *) &username));
>  	    }
> +	    FailedLogin (d, username);
>  	    RUN_AND_CHECK_PAM_ERROR(pam_end,
>  				    (*pamhp, pam_error));
>  	}
> @@ -638,7 +630,11 @@ greet_user_rtn GreetUser(
>  	if (Verify (d, greet, verify))
>  	    break;
>  	else
> -	    FailedLogin (d, greet);
> +	{
> +	    FailedLogin (d, greet->name);
> +	    bzero (greet->name, strlen(greet->name));
> +	    bzero (greet->password, strlen(greet->password));
> +	}
>  #endif
>      }
>      DeleteXloginResources (d, *dpy);