Bugs in ld-collate.c?

[Isamu Hasegawa <isamu at yamato dot ibm dot com>]

Hello again,

In locale/programs/ld-collate.c, line.2459...2502:

>	  if (elem_table[idx * 2] != 0)
>	    {
>	      /* The spot is already take.  Try iterating using the value
>		 from the secondary hashing function.  */
  [snip]
>	      while (elem_table[idx * 2] != 0);
>
>	      /* This is the spot where we will insert the value.  */
  [snip]
>	      obstack_int32_grow (&extrapool, runp->wcseqorder);
>	    }

In case of (elem_table[idx * 2] == 0), current collating element
isn't writed to elem_table.  I think if clause should close at just
after do-while clause.

And in line.2452:

>       if (runp->mbs != NULL && runp->weights != NULL)

I'm not sure, but I think characters are also exclude from elem_table,
because the max size of elem_size is 257.  Then the condition should
be like this?
  if (runp->mbs != NULL && runp->weights != NULL && !runp->is_character)

If my interpretations are correct, how about the attached patch?
# It seems to be a bit longer, but it is because of just reformating.

Changelog entry:

2001-08-09  Isamu Hasegawa  <isamu@yamato.ibm.com>

	* locale/programs/ld-collate.c (collate_output): Exclude
	characters from elem_table.
	Reduce if clause to write collating elements correctly.

Thanks,
-- 
Isamu Hasegawa
IBM Japan, Ltd.

Index: locale/programs/ld-collate.c
===================================================================
RCS file: /cvs/glibc/libc/locale/programs/ld-collate.c,v
retrieving revision 1.96
diff -u -r1.96 ld-collate.c
--- ld-collate.c	2001/08/04 05:12:33	1.96
+++ ld-collate.c	2001/08/09 09:48:39
@@ -2449,7 +2449,7 @@
   runp = collate->start;
   while (runp != NULL)
     {
-      if (runp->mbs != NULL && runp->weights != NULL)
+      if (runp->mbs != NULL && runp->weights != NULL && !runp->is_character)
 	{
 	  /* Compute the hash value of the name.  */
 	  uint32_t namelen = strlen (runp->name);
@@ -2469,37 +2469,36 @@
 		    idx -= elem_size;
 		}
 	      while (elem_table[idx * 2] != 0);
-
-	      /* This is the spot where we will insert the value.  */
-	      elem_table[idx * 2] = hash;
-	      elem_table[idx * 2 + 1] = obstack_object_size (&extrapool);
-
-	      /* The the string itself including length.  */
-	      obstack_1grow (&extrapool, namelen);
-	      obstack_grow (&extrapool, runp->name, namelen);
-
-	      /* And the multibyte representation.  */
-	      obstack_1grow (&extrapool, runp->nmbs);
-	      obstack_grow (&extrapool, runp->mbs, runp->nmbs);
-
-	      /* And align again to 32 bits.  */
-	      if ((1 + namelen + 1 + runp->nmbs) % sizeof (int32_t) != 0)
-		obstack_grow (&extrapool, "\0\0",
-			      (sizeof (int32_t)
-			       - ((1 + namelen + 1 + runp->nmbs)
-				  % sizeof (int32_t))));
-
-	      /* Now some 32-bit values: multibyte collation sequence,
-		 wide char string (including length), and wide char
-		 collation sequence.  */
-	      obstack_int32_grow (&extrapool, runp->mbseqorder);
-
-	      obstack_int32_grow (&extrapool, runp->nwcs);
-	      obstack_grow (&extrapool, runp->wcs,
-			    runp->nwcs * sizeof (uint32_t));
-
-	      obstack_int32_grow (&extrapool, runp->wcseqorder);
 	    }
+	  /* This is the spot where we will insert the value.  */
+ 	  elem_table[idx * 2] = hash;
+	  elem_table[idx * 2 + 1] = obstack_object_size (&extrapool);
+
+	  /* The the string itself including length.  */
+	  obstack_1grow (&extrapool, namelen);
+	  obstack_grow (&extrapool, runp->name, namelen);
+
+	  /* And the multibyte representation.  */
+	  obstack_1grow (&extrapool, runp->nmbs);
+	  obstack_grow (&extrapool, runp->mbs, runp->nmbs);
+
+	  /* And align again to 32 bits.  */
+	  if ((1 + namelen + 1 + runp->nmbs) % sizeof (int32_t) != 0)
+	    obstack_grow (&extrapool, "\0\0",
+			  (sizeof (int32_t)
+			   - ((1 + namelen + 1 + runp->nmbs)
+			      % sizeof (int32_t))));
+
+	  /* Now some 32-bit values: multibyte collation sequence,
+	     wide char string (including length), and wide char
+	     collation sequence.  */
+	  obstack_int32_grow (&extrapool, runp->mbseqorder);
+
+	  obstack_int32_grow (&extrapool, runp->nwcs);
+	  obstack_grow (&extrapool, runp->wcs,
+			runp->nwcs * sizeof (uint32_t));
+
+	  obstack_int32_grow (&extrapool, runp->wcseqorder);
 	}
 
       runp = runp->next;