[MIPS] Fix uninitialized variable in inet/getnetgrent_r.c

  On Wed, 2014-12-10 at 21:33 +0000, Joseph Myers wrote:
> On Wed, 10 Dec 2014, Steve Ellcey  wrote:
> 
> > I looked at the code and I don't think we can actually use an uninitialized
> > fct variable (due to the use of the no_more variable) but the compiler doesn't
> > seem to be able to figure that out.  This fix is to just initialize fct to
> > NULL.
> 
> As previously discussed, we don't want to add such initializations to 
> quiet warnings.
> 
> In this case, it looks like moving the while loop inside the "if (! 
> no_more)" ought to make it obvious to the compiler that fct can't be used 
> uninitialized.

OK, Here is a new patch that puts the while loop inside the if
statement.  That does get rid of the warning.  Other then indenting
changes (and tweaking a comment so it doesn't wrap) that is the only
change.

Tested on MIPS with no regressions.  OK for checkin?

Steve Ellcey
sellcey@imgtec.com


2014-12-11  Steve Ellcey  <sellcey@imgtec.com>

	* inet/getnetgrent_r.c: Move while loop to be inside if statement.
  

Hi,

i get the same warning/error on s390 if build with latest gcc.
This patch fixes the warning.

No new testsuite failure.

Thanks.
Stefan

On 12/11/2014 06:38 PM, Steve Ellcey wrote:
> On Wed, 2014-12-10 at 21:33 +0000, Joseph Myers wrote:
>> On Wed, 10 Dec 2014, Steve Ellcey  wrote:
>>
>>> I looked at the code and I don't think we can actually use an uninitialized
>>> fct variable (due to the use of the no_more variable) but the compiler doesn't
>>> seem to be able to figure that out.  This fix is to just initialize fct to
>>> NULL.
>>
>> As previously discussed, we don't want to add such initializations to
>> quiet warnings.
>>
>> In this case, it looks like moving the while loop inside the "if (!
>> no_more)" ought to make it obvious to the compiler that fct can't be used
>> uninitialized.
>
> OK, Here is a new patch that puts the while loop inside the if
> statement.  That does get rid of the warning.  Other then indenting
> changes (and tweaking a comment so it doesn't wrap) that is the only
> change.
>
> Tested on MIPS with no regressions.  OK for checkin?
>
> Steve Ellcey
> sellcey@imgtec.com
>
>
> 2014-12-11  Steve Ellcey  <sellcey@imgtec.com>
>
> 	* inet/getnetgrent_r.c: Move while loop to be inside if statement.
>
>
> diff --git a/inet/getnetgrent_r.c b/inet/getnetgrent_r.c
> index e101537..1f12ce9 100644
> --- a/inet/getnetgrent_r.c
> +++ b/inet/getnetgrent_r.c
> @@ -281,8 +281,8 @@ __internal_getnetgrent_r (char **hostp, char **userp, char **domainp,
>       {
>   #ifdef USE_NSCD
>         /* This bogus function pointer is a special marker left by
> -         __nscd_setnetgrent to tell us to use the data it left
> -         before considering any modules.  */
> +	 __nscd_setnetgrent to tell us to use the data it left
> +	 before considering any modules.  */
>         if (datap->nip == (service_user *) -1l)
>   	fct = nscd_getnetgrent;
>         else
> @@ -291,74 +291,73 @@ __internal_getnetgrent_r (char **hostp, char **userp, char **domainp,
>   	  fct = __nss_lookup_function (datap->nip, "getnetgrent_r");
>   	  no_more = fct == NULL;
>   	}
> -    }
> -
> -  while (! no_more)
> -    {
> -      status = DL_CALL_FCT (*fct, (datap, buffer, buflen, &errno));
>
> -      if (status == NSS_STATUS_RETURN
> -	  /* The service returned a NOTFOUND, but there are more groups that we
> -	     need to resolve before we give up.  */
> -	  || (status == NSS_STATUS_NOTFOUND && datap->needed_groups != NULL))
> +      while (! no_more)
>   	{
> -	  /* This was the last one for this group.  Look at next group
> -	     if available.  */
> -	  int found = 0;
> -	  while (datap->needed_groups != NULL && ! found)
> +	  status = DL_CALL_FCT (*fct, (datap, buffer, buflen, &errno));
> +
> +	  if (status == NSS_STATUS_RETURN
> +	      /* The service returned a NOTFOUND, but there are more groups that
> +		 we need to resolve before we give up.  */
> +	      || (status == NSS_STATUS_NOTFOUND && datap->needed_groups != NULL))
>   	    {
> -	      struct name_list *tmp = datap->needed_groups;
> -	      datap->needed_groups = datap->needed_groups->next;
> -	      tmp->next = datap->known_groups;
> -	      datap->known_groups = tmp;
> +	      /* This was the last one for this group.  Look at next group
> +		 if available.  */
> +	      int found = 0;
> +	      while (datap->needed_groups != NULL && ! found)
> +		{
> +		  struct name_list *tmp = datap->needed_groups;
> +		  datap->needed_groups = datap->needed_groups->next;
> +		  tmp->next = datap->known_groups;
> +		  datap->known_groups = tmp;
>
> -	      found = __internal_setnetgrent_reuse (datap->known_groups->name,
> -						    datap, errnop);
> -	    }
> +		  found = __internal_setnetgrent_reuse (datap->known_groups->name,
> +							datap, errnop);
> +		}
>
> -	  if (found && datap->nip != NULL)
> -	    {
> -	      fct = __nss_lookup_function (datap->nip, "getnetgrent_r");
> -	      if (fct != NULL)
> -		continue;
> +	      if (found && datap->nip != NULL)
> +		{
> +		  fct = __nss_lookup_function (datap->nip, "getnetgrent_r");
> +		  if (fct != NULL)
> +		    continue;
> +		}
>   	    }
> -	}
> -      else if (status == NSS_STATUS_SUCCESS && datap->type == group_val)
> -	{
> -	  /* The last entry was a name of another netgroup.  */
> -	  struct name_list *namep;
> -
> -	  /* Ignore if we've seen the name before.  */
> -	  for (namep = datap->known_groups; namep != NULL;
> -	       namep = namep->next)
> -	    if (strcmp (datap->val.group, namep->name) == 0)
> -	      break;
> -	  if (namep == NULL)
> -	    for (namep = datap->needed_groups; namep != NULL;
> -		 namep = namep->next)
> -	      if (strcmp (datap->val.group, namep->name) == 0)
> -		break;
> -	  if (namep != NULL)
> -	    /* Really ignore.  */
> -	    continue;
> -
> -	  size_t group_len = strlen (datap->val.group) + 1;
> -	  namep = (struct name_list *) malloc (sizeof (struct name_list)
> -					       + group_len);
> -	  if (namep == NULL)
> -	    /* We are out of memory.  */
> -	    status = NSS_STATUS_RETURN;
> -	  else
> +	  else if (status == NSS_STATUS_SUCCESS && datap->type == group_val)
>   	    {
> -	      namep->next = datap->needed_groups;
> -	      memcpy (namep->name, datap->val.group, group_len);
> -	      datap->needed_groups = namep;
> -	      /* And get the next entry.  */
> -	      continue;
> +	      /* The last entry was a name of another netgroup.  */
> +	      struct name_list *namep;
> +
> +	      /* Ignore if we've seen the name before.  */
> +	      for (namep = datap->known_groups; namep != NULL;
> +		   namep = namep->next)
> +		if (strcmp (datap->val.group, namep->name) == 0)
> +		  break;
> +	      if (namep == NULL)
> +		for (namep = datap->needed_groups; namep != NULL;
> +		     namep = namep->next)
> +		  if (strcmp (datap->val.group, namep->name) == 0)
> +		    break;
> +	      if (namep != NULL)
> +		/* Really ignore.  */
> +		continue;
> +
> +	      size_t group_len = strlen (datap->val.group) + 1;
> +	      namep = (struct name_list *) malloc (sizeof (struct name_list)
> +						  + group_len);
> +	      if (namep == NULL)
> +		/* We are out of memory.  */
> +		status = NSS_STATUS_RETURN;
> +	      else
> +		{
> +		  namep->next = datap->needed_groups;
> +		  memcpy (namep->name, datap->val.group, group_len);
> +		  datap->needed_groups = namep;
> +		  /* And get the next entry.  */
> +		  continue;
> +		}
>   	    }
> +	  break;
>   	}
> -
> -      break;
>       }
>
>     if (status == NSS_STATUS_SUCCESS)
> @@ -382,7 +381,7 @@ __getnetgrent_r (char **hostp, char **userp, char **domainp,
>     __libc_lock_lock (lock);
>
>     status = __internal_getnetgrent_r (hostp, userp, domainp, &dataset,
> -                                     buffer, buflen, &errno);
> +				     buffer, buflen, &errno);
>
>     __libc_lock_unlock (lock);
>
>
>
>
  

On Thu, 2014-12-11 at 09:38 -0800, Steve Ellcey wrote:
> On Wed, 2014-12-10 at 21:33 +0000, Joseph Myers wrote:
> > On Wed, 10 Dec 2014, Steve Ellcey  wrote:
> > 
> > > I looked at the code and I don't think we can actually use an uninitialized
> > > fct variable (due to the use of the no_more variable) but the compiler doesn't
> > > seem to be able to figure that out.  This fix is to just initialize fct to
> > > NULL.
> > 
> > As previously discussed, we don't want to add such initializations to 
> > quiet warnings.
> > 
> > In this case, it looks like moving the while loop inside the "if (! 
> > no_more)" ought to make it obvious to the compiler that fct can't be used 
> > uninitialized.
> 
> OK, Here is a new patch that puts the while loop inside the if
> statement.  That does get rid of the warning.  Other then indenting
> changes (and tweaking a comment so it doesn't wrap) that is the only
> change.
> 
> Tested on MIPS with no regressions.  OK for checkin?

OK.