Re: [gdm-list] [PATCH] fix gcc warnings

[Ludwig Nussel <ludwig nussel suse de>]

[reply moved here because I think you are referring to the patch in
this thread]

Brian Cameron wrote:
>Ludwig Nussel wrote:
>>Brian Cameron wrote:
>>> Anyway, I did not apply a few of your changes to daemon/verify-pam.c
>>> since they caused warnings to show up in Solaris.  If you want to
>>> rework that patch so that the current code is surrounded by
>>>
>>> #ifdef __sun
>>> ...
>>> #else
>>> your code
>>> #endif
>>>
>>> Then I would accept it, so it works on Solaris and Linux.
>> 
>> Which parts? I don't have Solaris so I can't check myself.
>
>Your previous patch only had five 1-2 line modifications.  The
>existing code should go in the #ifdef __sun section, your modification
>should go in the else.  I will test your patch on Solaris if you
>send it to me.

I assume you are referring to the following patch. If that causes
warnings on Solaris then the prototypes for conversation function
and pam_set_item must be different. On Linux it's

int (*conv)(int num_msg, const struct pam_message **msg, struct pam_response **resp, void *appdata_ptr);
int pam_get_item(const pam_handle_t *pamh, int item_type, const void **item);

I'd rather live with the warnings on either platform instead of
adding ugly #ifdefs

> Index: gdm2/daemon/verify-pam.c
> ===================================================================
> --- gdm2.orig/daemon/verify-pam.c
> +++ gdm2/daemon/verify-pam.c
> @@ -468,14 +468,14 @@ perhaps_translate_message (const char *m
>  /* Internal PAM conversation function. Interfaces between the PAM
>   * authentication system and the actual greeter program */
>  
> -static gint 
> -gdm_verify_pam_conv (int num_msg, struct pam_message **msg,
> +static int 
> +gdm_verify_pam_conv (int num_msg, const struct pam_message **msg,
>  		     struct pam_response **resp,
>  		     void *appdata_ptr)
>  {
>      int replies = 0;
>      int i;
> -    char *s;
> +    char *s = NULL;
>      struct pam_response *reply = NULL;
>      const void *p;
>      const char *login;
> @@ -501,7 +501,7 @@ gdm_verify_pam_conv (int num_msg, struct
>      /* Here we set the login if it wasn't already set,
>       * this is kind of anal, but this way we guarantee that
>       * the greeter always is up to date on the login */
> -    if (pam_get_item (pamh, PAM_USER, (void **)&p) == PAM_SUCCESS) {
> +    if (pam_get_item (pamh, PAM_USER, &p) == PAM_SUCCESS) {
>  	    login = (const char *)p;
>  	    gdm_slave_greeter_ctl_no_ret (GDM_SETLOGIN, login);
>      }
> @@ -603,8 +603,8 @@ static struct pam_conv pamc = {
>  /* Extra message to give on queries */
>  static char *extra_standalone_message = NULL;
>  
> -static gint 
> -gdm_verify_standalone_pam_conv (int num_msg, struct pam_message **msg,
> +static int 
> +gdm_verify_standalone_pam_conv (int num_msg, const struct pam_message **msg,
>  				struct pam_response **resp,
>  				void *appdata_ptr)
>  {
> @@ -929,7 +929,7 @@ authenticate_again:
>      g_free (login);
>      login = NULL;
>      
> -    if ((pamerr = pam_get_item (pamh, PAM_USER, (void **)&p)) != PAM_SUCCESS) {
> +    if ((pamerr = pam_get_item (pamh, PAM_USER, &p)) != PAM_SUCCESS) {
>  	    login = NULL;
>  	    /* is not really an auth problem, but it will
>  	       pretty much look as such, it shouldn't really
> @@ -1249,7 +1249,7 @@ gdm_verify_setup_user (GdmDisplay *d, co
>  	    goto setup_pamerr;
>      }
>  
> -    if ((pamerr = pam_get_item (pamh, PAM_USER, (void **)&p)) != PAM_SUCCESS) {
> +    if ((pamerr = pam_get_item (pamh, PAM_USER, &p)) != PAM_SUCCESS) {
>  	    /* is not really an auth problem, but it will
>  	       pretty much look as such, it shouldn't really
>  	       happen */

cu
Ludwig

-- 
 (o_   Ludwig Nussel
 //\   SUSE LINUX Products GmbH, Development
 V_/_  http://www.suse.de/