Re: Possible bug in sysdeps/unix/sysv/linux/powerpc/mmap64.c

["H . J . Lu" <hjl at lucon dot org>]

On Sat, May 19, 2001 at 04:01:59PM -0700, Ulrich Drepper wrote:
> "H . J . Lu" <hjl@lucon.org> writes:
> 
> > Here is a proposed patch.
> 
> Unless somebody who's responsible for the kernel part that the page
> size can be changed there is no need for a patch like this.  If it is

It has at least 3 problems:

1. Even on PPC, the kernel page size may change.
2. Linux may have variable page size one day.
3. sysdeps/unix/sysv/linux/powerpc/mmap64.c is included by
sysdeps/unix/sysv/linux/hppa/mmap64.c and other ports may do the
same.

> agreed on that it is necessary this
> 
> > +  if (!page_size)
> >      {
> > +      page_size = __getpagesize ();
> > +      page_shift = __ffs (page_size) - 1;
> > +    }
> > +
> > +  if (offset & (page_size - 1))
> 
> should be rewritten to store the page size minus one in a variable
> instead of computing it every time again.
> 

Like this?


H.J.
---
Index: sysdeps/unix/sysv/linux/powerpc/mmap64.c
===================================================================
RCS file: /work/cvs/gnu/glibc/sysdeps/unix/sysv/linux/powerpc/mmap64.c,v
retrieving revision 1.1.1.5
diff -u -p -r1.1.1.5 mmap64.c
--- sysdeps/unix/sysv/linux/powerpc/mmap64.c	2001/05/05 04:06:28	1.1.1.5
+++ sysdeps/unix/sysv/linux/powerpc/mmap64.c	2001/05/19 23:12:41
@@ -18,6 +18,7 @@
    Boston, MA 02111-1307, USA.  */
 
 #include <errno.h>
+#include <string.h>
 #include <unistd.h>
 #include <sys/mman.h>
 
@@ -27,9 +28,8 @@
 
 #include "kernel-features.h"
 
-/* This is always the constant 12 for this routine, even if the actual
-   page size is larger.  */
-#define PAGE_SHIFT 12
+static int page_shift;
+static int page_mask;
 
 #ifdef __NR_mmap2
 extern void *__unbounded __syscall_mmap2(void *__unbounded, size_t,
@@ -42,8 +42,15 @@ static int have_no_mmap2;
 void *
 __mmap64 (void *addr, size_t len, int prot, int flags, int fd, off64_t offset)
 {
-  if (offset & ((1 << PAGE_SHIFT)-1))
+  if (!page_mask)
     {
+      page_mask = __getpagesize ();
+      page_shift = __ffs (page_mask) - 1;
+      page_mask--;
+    }
+
+  if (offset & page_mask)
+    {
       __set_errno (EINVAL);
       return MAP_FAILED;
     }
@@ -58,7 +65,7 @@ __mmap64 (void *addr, size_t len, int pr
       void *result;
       __ptrvalue (result) = INLINE_SYSCALL (mmap2, 6, __ptrvalue (addr), len,
 					    prot, flags, fd,
-					    (off_t) (offset >> PAGE_SHIFT));
+					    (off_t) (offset >> page_shift));
 # if __BOUNDED_POINTERS__
       __ptrlow (result) = __ptrvalue (result);
       __ptrhigh (result) = __ptrvalue (result) + len;