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: locale files and int32_t alignment


On Mon, 23 Sep 2013, Roland McGrath wrote:

> I think the change is good in principle.  But it should be more explicit in
> the code.  A few macros (e.g. LOCFILE_ALIGN, LOCFILE_ALIGNED_P (x)) instead
> of just sizeof (int32_t) all over would serve as adequate implicit
> documentation.

Here is a patch that defines such macros and uses them *only* in the
cases where __alignof__ (int32_t) was used (on the basis that the
substantive change, that actually affects the format of locale files
on m68k, should be separate from pure cleanup changes making other
code with hardcoded constants such as "3" or "4" use these macros).

Tested x86_64.  As before, you don't get identical localedef
disassembly because of the changes to strings in assertions, but
libc.so.6 disassembly is unchanged, as is the generated
locale-archive.

2013-09-26  Joseph Myers  <joseph@codesourcery.com>

	* locale/localeinfo.h (LOCFILE_ALIGN): New macro.
	(LOCFILE_ALIGN_MASK): Likewise.
	(LOCFILE_ALIGN_UP): Likewise.
	(LOCFILE_ALIGNED_P): Likewise.
	* locale/programs/ld-collate.c (collate_output): Use the new
	macros instead of __alignof__ (int32_t).
	* locale/weight.h (findidx): Likewise.

diff --git a/locale/localeinfo.h b/locale/localeinfo.h
index 3142726..8d2c166 100644
--- a/locale/localeinfo.h
+++ b/locale/localeinfo.h
@@ -87,6 +87,16 @@ struct __locale_data
   values __flexarr;	/* Items, usually pointers into `filedata'.  */
 };
 
+/* This alignment is used for 32-bit integers in locale files, both
+   those that are explicitly int32_t or uint32_t and those that are
+   wchar_t, regardless of the (possibly smaller) alignment required
+   for such integers on a particular host.  */
+#define LOCFILE_ALIGN		sizeof (int32_t)
+#define LOCFILE_ALIGN_MASK	(LOCFILE_ALIGN - 1)
+#define LOCFILE_ALIGN_UP(x)	(((x) + LOCFILE_ALIGN - 1)	\
+				 & ~LOCFILE_ALIGN_MASK)
+#define LOCFILE_ALIGNED_P(x)	(((x) & LOCFILE_ALIGN_MASK) == 0)
+
 /* We know three kinds of collation sorting rules.  */
 enum coll_sort_rule
 {
diff --git a/locale/programs/ld-collate.c b/locale/programs/ld-collate.c
index e610389..31e2d05 100644
--- a/locale/programs/ld-collate.c
+++ b/locale/programs/ld-collate.c
@@ -2154,11 +2154,11 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 	++i;
       }
   /* And align the output.  */
-  i = (nrules * i) % __alignof__ (int32_t);
+  i = (nrules * i) % LOCFILE_ALIGN;
   if (i > 0)
     do
       obstack_1grow (&weightpool, '\0');
-    while (++i < __alignof__ (int32_t));
+    while (++i < LOCFILE_ALIGN);
 
   add_locale_raw_obstack (&file, &weightpool);
 
@@ -2204,8 +2204,7 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 	struct element_t *runp = collate->mbheads[ch];
 	struct element_t *lastp;
 
-	assert ((obstack_object_size (&extrapool)
-		 & (__alignof__ (int32_t) - 1)) == 0);
+	assert (LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)));
 
 	tablemb[ch] = -obstack_object_size (&extrapool);
 
@@ -2230,11 +2229,9 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 		struct element_t *curp;
 
 		/* Compute how much space we will need.  */
-		added = ((sizeof (int32_t) + 1 + 2 * (runp->nmbs - 1)
-			  + __alignof__ (int32_t) - 1)
-			 & ~(__alignof__ (int32_t) - 1));
-		assert ((obstack_object_size (&extrapool)
-			 & (__alignof__ (int32_t) - 1)) == 0);
+		added = LOCFILE_ALIGN_UP (sizeof (int32_t) + 1
+					  + 2 * (runp->nmbs - 1));
+		assert (LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)));
 		obstack_make_room (&extrapool, added);
 
 		/* More than one consecutive entry.  We mark this by having
@@ -2291,11 +2288,9 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 		/* Output the weight info.  */
 		weightidx = output_weight (&weightpool, collate, runp);
 
-		added = ((sizeof (int32_t) + 1 + runp->nmbs - 1
-			  + __alignof__ (int32_t) - 1)
-			 & ~(__alignof__ (int32_t) - 1));
-		assert ((obstack_object_size (&extrapool)
-			 & (__alignof__ (int32_t) - 1)) == 0);
+		added = LOCFILE_ALIGN_UP (sizeof (int32_t) + 1
+					  + runp->nmbs - 1);
+		assert (LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)));
 		obstack_make_room (&extrapool, added);
 
 		obstack_int32_grow_fast (&extrapool, weightidx);
@@ -2307,8 +2302,7 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 	      }
 
 	    /* Add alignment bytes if necessary.  */
-	    while ((obstack_object_size (&extrapool)
-		    & (__alignof__ (int32_t) - 1)) != 0)
+	    while (!LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)))
 	      obstack_1grow_fast (&extrapool, '\0');
 
 	    /* Next entry.  */
@@ -2317,15 +2311,13 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 	  }
 	while (runp != NULL);
 
-	assert ((obstack_object_size (&extrapool)
-		 & (__alignof__ (int32_t) - 1)) == 0);
+	assert (LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)));
 
 	/* If the final entry in the list is not a single character we
 	   add an UNDEFINED entry here.  */
 	if (lastp->nmbs != 1)
 	  {
-	    int added = ((sizeof (int32_t) + 1 + 1 + __alignof__ (int32_t) - 1)
-			 & ~(__alignof__ (int32_t) - 1));
+	    int added = LOCFILE_ALIGN_UP (sizeof (int32_t) + 1 + 1);
 	    obstack_make_room (&extrapool, added);
 
 	    obstack_int32_grow_fast (&extrapool, 0);
@@ -2335,15 +2327,13 @@ collate_output (struct localedef_t *locale, const struct charmap_t *charmap,
 	    obstack_1grow_fast (&extrapool, 0);
 
 	    /* Add alignment bytes if necessary.  */
-	    while ((obstack_object_size (&extrapool)
-		    & (__alignof__ (int32_t) - 1)) != 0)
+	    while (!LOCFILE_ALIGNED_P (obstack_object_size (&extrapool)))
 	      obstack_1grow_fast (&extrapool, '\0');
 	  }
       }
 
   /* Add padding to the tables if necessary.  */
-  while ((obstack_object_size (&weightpool) & (__alignof__ (int32_t) - 1))
-	 != 0)
+  while (!LOCFILE_ALIGNED_P (obstack_object_size (&weightpool)))
     obstack_1grow (&weightpool, 0);
 
   /* Now add the four tables.  */
diff --git a/locale/weight.h b/locale/weight.h
index 645eda2..b097aac 100644
--- a/locale/weight.h
+++ b/locale/weight.h
@@ -69,8 +69,8 @@ findidx (const unsigned char **cpp, size_t len)
 
 	  /* Up to the next entry.  */
 	  cp += nhere;
-	  if ((1 + nhere) % __alignof__ (int32_t) != 0)
-	    cp += __alignof__ (int32_t) - (1 + nhere) % __alignof__ (int32_t);
+	  if (!LOCFILE_ALIGNED_P (1 + nhere))
+	    cp += LOCFILE_ALIGN - (1 + nhere) % LOCFILE_ALIGN;
 	}
       else
 	{
@@ -89,9 +89,9 @@ findidx (const unsigned char **cpp, size_t len)
 		{
 		  /* Cannot be in this range.  */
 		  cp += 2 * nhere;
-		  if ((1 + 2 * nhere) % __alignof__ (int32_t) != 0)
-		    cp += (__alignof__ (int32_t)
-			   - (1 + 2 * nhere) % __alignof__ (int32_t));
+		  if (!LOCFILE_ALIGNED_P (1 + 2 * nhere))
+		    cp += (LOCFILE_ALIGN
+			   - (1 + 2 * nhere) % LOCFILE_ALIGN);
 		  continue;
 		}
 
@@ -104,9 +104,9 @@ findidx (const unsigned char **cpp, size_t len)
 		{
 		  /* Cannot be in this range.  */
 		  cp += 2 * nhere;
-		  if ((1 + 2 * nhere) % __alignof__ (int32_t) != 0)
-		    cp += (__alignof__ (int32_t)
-			   - (1 + 2 * nhere) % __alignof__ (int32_t));
+		  if (!LOCFILE_ALIGNED_P (1 + 2 * nhere))
+		    cp += (LOCFILE_ALIGN
+			   - (1 + 2 * nhere) % LOCFILE_ALIGN);
 		  continue;
 		}
 

-- 
Joseph S. Myers
joseph@codesourcery.com


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