This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Patch, MIPS] Fix uninitialized variable in inet/getnetgrent_r.c


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);






Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]