diff mbox

[v2] Fix -Wundef warning on PAGE_COPY_THRESHOLD

Message ID 20140702074857.GD20796@spoyarek.pnq.redhat.com
State Committed
Headers show

Commit Message

Siddhesh Poyarekar July 2, 2014, 7:48 a.m. UTC
On Tue, Jul 01, 2014 at 10:47:30AM -0700, Roland McGrath wrote:
> Generally memcopy.h is machine-specific while pagecopy.h is OS-specific.
> Any scheme is OK with me if it follows typo-proof principles and means
> that an OS-specific header can use #include_next and #undef+#define to
> get all the rights bits without any source code duplication.

It was indeed easier and nicer to do what you said, i.e. define
PAGE_COPY_THRESHOLD in pagecopy.h for system-specific pagecopy.h to
implement and then define PAGE_COPY_FWD_MAYBE in memcopy.h.  I also
included pagecopy.h in memcopy.h since only including pagecopy.h is
never going to be a useful use case now.

Siddhesh

	* debug/memcpy_chk.c: Don't include pagecopy.h.
	* debug/mempcpy_chk.c: Likewise.
	* string/memcpy.c: Likewise.
	* string/memmove.c: Likewise.
	* sysdeps/powerpc/memmove.c: Likewise.
	* sysdeps/generic/memcopy.h: Include pagecopy.h.  Move
	definition of PAGE_COPY_FWD_MAYBE here...
	* sysdeps/generic/pagecopy.h: ... from here.
	* sysdeps/mach/pagecopy.h: Don't include generic pagecopy.h.

Comments

Roland McGrath July 2, 2014, 4:19 p.m. UTC | #1
Looks OK to me.
diff mbox

Patch

diff --git a/debug/memcpy_chk.c b/debug/memcpy_chk.c
index 9bf5d9f..539b7e2 100644
--- a/debug/memcpy_chk.c
+++ b/debug/memcpy_chk.c
@@ -20,7 +20,6 @@ 
 
 #include <string.h>
 #include <memcopy.h>
-#include <pagecopy.h>
 
 void *
 __memcpy_chk (dstpp, srcpp, len, dstlen)
diff --git a/debug/mempcpy_chk.c b/debug/mempcpy_chk.c
index 105356f..3ca916b 100644
--- a/debug/mempcpy_chk.c
+++ b/debug/mempcpy_chk.c
@@ -21,7 +21,6 @@ 
 
 #include <string.h>
 #include <memcopy.h>
-#include <pagecopy.h>
 
 void *
 __mempcpy_chk (dstpp, srcpp, len, dstlen)
diff --git a/string/memcpy.c b/string/memcpy.c
index c19bad3..517f9ee 100644
--- a/string/memcpy.c
+++ b/string/memcpy.c
@@ -20,7 +20,6 @@ 
 
 #include <string.h>
 #include <memcopy.h>
-#include <pagecopy.h>
 
 #undef memcpy
 
diff --git a/string/memmove.c b/string/memmove.c
index 3373401..22140ef 100644
--- a/string/memmove.c
+++ b/string/memmove.c
@@ -20,7 +20,6 @@ 
 
 #include <string.h>
 #include <memcopy.h>
-#include <pagecopy.h>
 
 /* All this is so that bcopy.c can #include
    this file after defining some things.  */
diff --git a/sysdeps/generic/memcopy.h b/sysdeps/generic/memcopy.h
index b7bd5e9..f7b9423 100644
--- a/sysdeps/generic/memcopy.h
+++ b/sysdeps/generic/memcopy.h
@@ -40,6 +40,7 @@ 
 
 #include <sys/cdefs.h>
 #include <endian.h>
+#include <pagecopy.h>
 
 /* The macros defined in this file are:
 
@@ -144,6 +145,47 @@  extern void _wordcopy_bwd_dest_aligned (long int, long int, size_t) __THROW;
       (nbytes_left) = (nbytes) % OPSIZ;					      \
     } while (0)
 
+/* The macro PAGE_COPY_FWD_MAYBE (dstp, srcp, nbytes_left, nbytes) is invoked
+   like WORD_COPY_FWD et al.  The pointers should be at least word aligned.
+   This will check if virtual copying by pages can and should be done and do it
+   if so.  The pointers will be aligned to PAGE_SIZE bytes.  The macro requires
+   that pagecopy.h defines at least PAGE_COPY_THRESHOLD to 0.  If
+   PAGE_COPY_THRESHOLD is non-zero, the header must also define PAGE_COPY_FWD
+   and PAGE_SIZE.
+*/
+#if PAGE_COPY_THRESHOLD
+
+# include <assert.h>
+
+# define PAGE_COPY_FWD_MAYBE(dstp, srcp, nbytes_left, nbytes)		      \
+  do									      \
+    {									      \
+      if ((nbytes) >= PAGE_COPY_THRESHOLD &&				      \
+	  PAGE_OFFSET ((dstp) - (srcp)) == 0) 				      \
+	{								      \
+	  /* The amount to copy is past the threshold for copying	      \
+	     pages virtually with kernel VM operations, and the		      \
+	     source and destination addresses have the same alignment.  */    \
+	  size_t nbytes_before = PAGE_OFFSET (-(dstp));			      \
+	  if (nbytes_before != 0)					      \
+	    {								      \
+	      /* First copy the words before the first page boundary.  */     \
+	      WORD_COPY_FWD (dstp, srcp, nbytes_left, nbytes_before);	      \
+	      assert (nbytes_left == 0);				      \
+	      nbytes -= nbytes_before;					      \
+	    }								      \
+	  PAGE_COPY_FWD (dstp, srcp, nbytes_left, nbytes);		      \
+	}								      \
+    } while (0)
+
+/* The page size is always a power of two, so we can avoid modulo division.  */
+# define PAGE_OFFSET(n)	((n) & (PAGE_SIZE - 1))
+
+#else
+
+# define PAGE_COPY_FWD_MAYBE(dstp, srcp, nbytes_left, nbytes) /* nada */
+
+#endif
 
 /* Threshold value for when to enter the unrolled loops.  */
 #define	OP_T_THRES	16
diff --git a/sysdeps/generic/pagecopy.h b/sysdeps/generic/pagecopy.h
index 2862270..3c81de1 100644
--- a/sysdeps/generic/pagecopy.h
+++ b/sysdeps/generic/pagecopy.h
@@ -1,4 +1,4 @@ 
-/* Macros for copying by pages; used in memcpy, memmove.  Generic macros.
+/* Macros for copying by pages; used in memcpy, memmove.
    Copyright (C) 1995-2014 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
@@ -16,19 +16,15 @@ 
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-/* This file defines the macro:
+/* The macro PAGE_COPY_FWD_MAYBE defined in memcopy.h is used in memmove if the
+   PAGE_COPY_THRESHOLD macro is set to a non-zero value.  The default is 0,
+   that is copying by pages is not implemented.
 
-   PAGE_COPY_FWD_MAYBE (dstp, srcp, nbytes_left, nbytes)
-
-   which is invoked like WORD_COPY_FWD et al.  The pointers should be at
-   least word aligned.  This will check if virtual copying by pages can and
-   should be done and do it if so.
-
-   System-specific pagecopy.h files should define these macros and then
-   #include this file:
+   System-specific pagecopy.h files that want to support page copying should
+   define these macros:
 
    PAGE_COPY_THRESHOLD
-   -- Minimum size for which virtual copying by pages is worthwhile.
+   -- A non-zero minimum size for which virtual copying by pages is worthwhile.
 
    PAGE_SIZE
    -- Size of a page.
@@ -38,37 +34,4 @@ 
    The pointers will be aligned to PAGE_SIZE bytes.
 */
 
-
-#if defined PAGE_COPY_THRESHOLD && PAGE_COPY_THRESHOLD
-
-#include <assert.h>
-
-#define PAGE_COPY_FWD_MAYBE(dstp, srcp, nbytes_left, nbytes)		      \
-  do									      \
-    {									      \
-      if ((nbytes) >= PAGE_COPY_THRESHOLD &&				      \
-	  PAGE_OFFSET ((dstp) - (srcp)) == 0) 				      \
-	{								      \
-	  /* The amount to copy is past the threshold for copying	      \
-	     pages virtually with kernel VM operations, and the		      \
-	     source and destination addresses have the same alignment.  */    \
-	  size_t nbytes_before = PAGE_OFFSET (-(dstp));			      \
-	  if (nbytes_before != 0)					      \
-	    {								      \
-	      /* First copy the words before the first page boundary.  */     \
-	      WORD_COPY_FWD (dstp, srcp, nbytes_left, nbytes_before);	      \
-	      assert (nbytes_left == 0);				      \
-	      nbytes -= nbytes_before;					      \
-	    }								      \
-	  PAGE_COPY_FWD (dstp, srcp, nbytes_left, nbytes);		      \
-	}								      \
-    } while (0)
-
-/* The page size is always a power of two, so we can avoid modulo division.  */
-#define PAGE_OFFSET(n)	((n) & (PAGE_SIZE - 1))
-
-#else
-
-#define PAGE_COPY_FWD_MAYBE(dstp, srcp, nbytes_left, nbytes) /* nada */
-
-#endif
+#define PAGE_COPY_THRESHOLD 0
diff --git a/sysdeps/mach/pagecopy.h b/sysdeps/mach/pagecopy.h
index 36c3697..8fa5555 100644
--- a/sysdeps/mach/pagecopy.h
+++ b/sysdeps/mach/pagecopy.h
@@ -31,5 +31,3 @@ 
 		     ? trunc_page (nbytes)				      \
 		     : 0)))
 
-/* Get the generic macro.  */
-#include <sysdeps/generic/pagecopy.h>
diff --git a/sysdeps/powerpc/memmove.c b/sysdeps/powerpc/memmove.c
index 3859da8..98ffb14 100644
--- a/sysdeps/powerpc/memmove.c
+++ b/sysdeps/powerpc/memmove.c
@@ -20,7 +20,6 @@ 
 
 #include <string.h>
 #include <memcopy.h>
-#include <pagecopy.h>
 
 /* All this is so that bcopy.c can #include
    this file after defining some things.  */