[prev in list] [next in list] [prev in thread] [next in thread] 

List:       apr-dev
Subject:    Re: svn commit: r1887060 - in /apr/apr/trunk: CHANGES include/apr_mmap.h mmap/unix/mmap.c test/data/
From:       Yann Ylavic <ylavic.dev () gmail ! com>
Date:       2021-03-02 13:11:47
Message-ID: CAKQ1sVNfULghrou_TnJWdehXcoQkxU+o3-raBdk1h6X6TpsiJQ () mail ! gmail ! com
[Download RAW message or body]

On Tue, Mar 2, 2021 at 8:35 AM Ruediger Pluem <rpluem@apache.org> wrote:
>
> On 3/2/21 1:54 AM, Yann Ylavic wrote:
> >
> > By adding this "poffset" member to apr_mmap_t (on unixes), its layout changes.
> > Can I backport this to 1.7 or should I find another way to store
> > "poffset" there (like some pool/cntxt userdata)?
>
> I don't think that this can be backported this way as it changes a non opaque struct on the public API.
> So you would need to look for a different area to store this data.

That's what I feared :)

Two proposals then (patches attached):

1. pool userdata
Should work for any apr_mmap_t, even if it's not created by apr_mmap_create().
It's slower on delete/cleanup though, and "leaks" a userdata key on
the pool for each mmap created.

2. layout struct { apr_mmap_t mm; apr_off_t poffset; } internal to
mmap/unix/mmap.c
This one is as effective as trunk's version, but requires that the
apr_mmap_t be apr_mmap_create()d (not allocated by the user or on the
stack).
It's not a problem with apr_mmap_delete() because it's a noop for an
handcrafted apr_mmap_t (no mmap_cleanup registered), however
apr_mmap_dup() may read above the limits..
How much do we consider valid an apr_mmap_t not apr_mmap_create()d?

Regards;
Yann.

["apr_mmap-1.7.x-poffset_userdata.diff" (text/x-patch)]

Index: CHANGES
===================================================================
--- CHANGES	(revision 1887062)
+++ CHANGES	(working copy)
@@ -1,6 +1,8 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 1.7.1
 
+  *) Align apr_mmap()ing offset to a page boundary.  PR 65158.  [Yann Ylavic]
+
   *) Don't silently set APR_FOPEN_NOCLEANUP for apr_file_mktemp() created file
      to avoid a fd and inode leak when/if later passed to apr_file_setaside().
      [Yann Ylavic]
Index: mmap/unix/mmap.c
===================================================================
--- mmap/unix/mmap.c	(revision 1887062)
+++ mmap/unix/mmap.c	(working copy)
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <assert.h>
+
 #include "apr.h"
 #include "apr_private.h"
 #include "apr_general.h"
@@ -33,6 +35,9 @@
 #if APR_HAVE_STDIO_H
 #include <stdio.h>
 #endif
+#if APR_HAVE_UNISTD_H
+#include <unistd.h>  /* for sysconf() */
+#endif
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
@@ -42,10 +47,31 @@
 
 #if APR_HAS_MMAP || defined(BEOS)
 
+#ifndef BEOS
+static void set_poffset(apr_mmap_t *mm, apr_off_t poffset)
+{
+    apr_pool_userdata_setn(apr_pmemdup(mm->cntxt, &poffset, sizeof(poffset)),
+                           apr_psprintf(mm->cntxt, "apr_mm_%pp", mm),
+                           NULL, mm->cntxt);
+}
+
+static apr_off_t get_poffset(apr_mmap_t *mm)
+{
+    char key[32];
+    apr_off_t *poffset = NULL;
+    apr_snprintf(key, sizeof(key), "apr_mm_%pp", mm);
+    apr_pool_userdata_get((void **)&poffset, key, mm->cntxt);
+    return poffset ? *poffset : 0;
+}
+#endif
+
 static apr_status_t mmap_cleanup(void *themmap)
 {
     apr_mmap_t *mm = themmap;
     apr_mmap_t *next = APR_RING_NEXT(mm,link);
+#ifndef BEOS
+    apr_off_t poffset;
+#endif
     int rv = 0;
 
     /* we no longer refer to the mmaped region */
@@ -61,7 +87,8 @@ static apr_status_t mmap_cleanup(void *themmap)
 #ifdef BEOS
     rv = delete_area(mm->area);
 #else
-    rv = munmap(mm->mm, mm->size);
+    poffset = get_poffset(mm);
+    rv = munmap((char *)mm->mm - poffset, mm->size + poffset);
 #endif
     mm->mm = (void *)-1;
 
@@ -81,6 +108,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
     area_id aid = -1;
     uint32 pages = 0;
 #else
+    static long psize;
+    apr_off_t poffset = 0;
     apr_int32_t native_flags = 0;
 #endif
 
@@ -130,8 +159,19 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
         native_flags |= PROT_READ;
     }
 
-    mm = mmap(NULL, size, native_flags, MAP_SHARED, file->filedes, offset);
+#if defined(_SC_PAGESIZE)
+    if (psize == 0) {
+        psize = sysconf(_SC_PAGESIZE);
+        /* the page size should be a power of two */
+        assert(psize > 0 && (psize & (psize - 1)) == 0);
+    }
+    poffset = offset & (apr_off_t)(psize - 1);
+#endif
 
+    mm = mmap(NULL, size + poffset,
+              native_flags, MAP_SHARED,
+              file->filedes, offset - poffset);
+
     if (mm == (void *)-1) {
         /* we failed to get an mmap'd file... */
         *new = NULL;
@@ -139,7 +179,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
     }
 #endif
 
-    (*new)->mm = mm;
+    (*new)->mm = (char *)mm + poffset;
+    set_poffset(*new, poffset);
     (*new)->size = size;
     (*new)->cntxt = cont;
     APR_RING_ELEM_INIT(*new, link);
Index: test/data/mmap_large_datafile.txt
===================================================================
Index: test/testmmap.c
===================================================================
--- test/testmmap.c	(revision 1887062)
+++ test/testmmap.c	(working copy)
@@ -35,7 +35,8 @@ static void not_implemented(abts_case *tc, void *d
 
 #else
 
-static char test_string[256]; /* read from the datafile */
+static apr_pool_t *ptest;
+static char *thisfdata; /* read from the datafile */
 static apr_mmap_t *themmap = NULL;
 static apr_file_t *thefile = NULL;
 static char *file1;
@@ -42,11 +43,22 @@ static char *file1;
 static apr_finfo_t thisfinfo;
 static apr_size_t thisfsize;
 
+static struct {
+    const char *filename;
+    apr_off_t offset;
+} test_set[] = {
+    { "/data/mmap_datafile.txt", 0 },
+    { "/data/mmap_large_datafile.txt", 65536 },
+    { "/data/mmap_large_datafile.txt", 66650 }, /* not page aligned */
+    { NULL, }
+};
+
 static void create_filename(abts_case *tc, void *data)
 {
+    const char *filename = data;
     char *oldfileptr;
 
-    apr_filepath_get(&file1, 0, p);
+    apr_filepath_get(&file1, 0, ptest);
 #ifndef NETWARE
 #ifdef WIN32
     ABTS_TRUE(tc, file1[1] == ':');
@@ -57,7 +69,7 @@ static void create_filename(abts_case *tc, void *d
     ABTS_TRUE(tc, file1[strlen(file1) - 1] != '/');
 
     oldfileptr = file1;
-    file1 = apr_pstrcat(p, file1,"/data/mmap_datafile.txt" ,NULL);
+    file1 = apr_pstrcat(ptest, file1, filename, NULL);
     ABTS_TRUE(tc, oldfileptr != file1);
 }
 
@@ -67,24 +79,15 @@ static void test_file_close(abts_case *tc, void *d
 
     rv = apr_file_close(thefile);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    thefile = NULL;
 }
    
-static void read_expected_contents(abts_case *tc, void *data)
-{
-    apr_status_t rv;
-    apr_size_t nbytes = sizeof(test_string) - 1;
-
-    rv = apr_file_read(thefile, test_string, &nbytes);
-    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
-    test_string[nbytes] = '\0';
-    thisfsize = strlen(test_string);
-}
-
 static void test_file_open(abts_case *tc, void *data)
 {
     apr_status_t rv;
 
-    rv = apr_file_open(&thefile, file1, APR_FOPEN_READ, APR_UREAD | APR_GREAD, p);
+    rv = apr_file_open(&thefile, file1, APR_FOPEN_READ,
+                       APR_UREAD | APR_GREAD, ptest);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
     ABTS_PTR_NOTNULL(tc, thefile);
 }
@@ -95,15 +98,42 @@ static void test_get_filesize(abts_case *tc, void
 
     rv = apr_file_info_get(&thisfinfo, APR_FINFO_NORM, thefile);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
-    ABTS_ASSERT(tc, "File size mismatch", thisfsize == thisfinfo.size);
+
+    thisfsize = thisfinfo.size;
+    thisfdata = apr_palloc(ptest, thisfsize + 1);
+    ABTS_PTR_NOTNULL(tc, thisfdata);
 }
 
+static void read_expected_contents(abts_case *tc, void *data)
+{
+    apr_off_t *offset = data;
+    apr_size_t nbytes = 0;
+    apr_status_t rv;
+
+    rv = apr_file_read_full(thefile, thisfdata, thisfsize, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_ASSERT(tc, "File size mismatch", nbytes == thisfsize);
+    thisfdata[nbytes] = '\0';
+    ABTS_ASSERT(tc, "File content size mismatch",
+                strlen(thisfdata) == thisfsize);
+    ABTS_ASSERT(tc, "File size too small",
+                (apr_size_t)*offset < thisfsize);
+
+    /* From now, pretend that the file data and size don't include the
+     * offset, this avoids adding/substrating it to thisfdata/thisfsize
+     * all over the place in the next tests.
+     */
+    thisfdata += *offset;
+    thisfsize -= *offset;
+}
+
 static void test_mmap_create(abts_case *tc, void *data)
 {
+    apr_off_t *offset = data;
     apr_status_t rv;
 
-    rv = apr_mmap_create(&themmap, thefile, 0, (apr_size_t) thisfinfo.size, 
-		                 APR_MMAP_READ, p);
+    rv = apr_mmap_create(&themmap, thefile, *offset, thisfsize,
+                         APR_MMAP_READ, ptest);
     ABTS_PTR_NOTNULL(tc, themmap);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 }
@@ -110,13 +140,12 @@ static void test_mmap_create(abts_case *tc, void *
 
 static void test_mmap_contents(abts_case *tc, void *data)
 {
-    
     ABTS_PTR_NOTNULL(tc, themmap);
     ABTS_PTR_NOTNULL(tc, themmap->mm);
     ABTS_SIZE_EQUAL(tc, thisfsize, themmap->size);
 
     /* Must use nEquals since the string is not guaranteed to be NULL terminated */
-    ABTS_STR_NEQUAL(tc, themmap->mm, test_string, thisfsize);
+    ABTS_STR_NEQUAL(tc, themmap->mm, thisfdata, thisfsize);
 }
 
 static void test_mmap_delete(abts_case *tc, void *data)
@@ -126,6 +155,7 @@ static void test_mmap_delete(abts_case *tc, void *
     ABTS_PTR_NOTNULL(tc, themmap);
     rv = apr_mmap_delete(themmap);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    themmap = NULL;
 }
 
 static void test_mmap_offset(abts_case *tc, void *data)
@@ -138,8 +168,9 @@ static void test_mmap_offset(abts_case *tc, void *
 
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
     /* Must use nEquals since the string is not guaranteed to be NULL terminated */
-    ABTS_STR_NEQUAL(tc, addr, test_string + 5, thisfsize-5);
+    ABTS_STR_NEQUAL(tc, addr, thisfdata + 5, thisfsize - 5);
 }
+
 #endif
 
 abts_suite *testmmap(abts_suite *suite)
@@ -147,15 +178,21 @@ abts_suite *testmmap(abts_suite *suite)
     suite = ADD_SUITE(suite)
 
 #if APR_HAS_MMAP    
-    abts_run_test(suite, create_filename, NULL);
-    abts_run_test(suite, test_file_open, NULL);
-    abts_run_test(suite, read_expected_contents, NULL);
-    abts_run_test(suite, test_get_filesize, NULL);
-    abts_run_test(suite, test_mmap_create, NULL);
-    abts_run_test(suite, test_mmap_contents, NULL);
-    abts_run_test(suite, test_mmap_offset, NULL);
-    abts_run_test(suite, test_mmap_delete, NULL);
-    abts_run_test(suite, test_file_close, NULL);
+    apr_size_t i;
+    apr_pool_create(&ptest, p);
+    for (i = 0; test_set[i].filename; ++i) {
+        abts_run_test(suite, create_filename, (void *)test_set[i].filename);
+        abts_run_test(suite, test_file_open, NULL);
+        abts_run_test(suite, test_get_filesize, NULL);
+        abts_run_test(suite, read_expected_contents, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_create, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_contents, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_offset, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_delete, NULL);
+        abts_run_test(suite, test_file_close, NULL);
+        apr_pool_clear(ptest);
+    }
+    apr_pool_destroy(ptest);
 #else
     abts_run_test(suite, not_implemented, NULL);
 #endif
Index: .
===================================================================
--- .	(revision 1887062)
+++ .	(working copy)

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /apr/apr/trunk:r1887060

["apr_mmap-1.7.x-poffset_layout.diff" (text/x-patch)]

Index: CHANGES
===================================================================
--- CHANGES	(revision 1887062)
+++ CHANGES	(working copy)
@@ -1,6 +1,8 @@
                                                      -*- coding: utf-8 -*-
 Changes for APR 1.7.1
 
+  *) Align apr_mmap()ing offset to a page boundary.  PR 65158.  [Yann Ylavic]
+
   *) Don't silently set APR_FOPEN_NOCLEANUP for apr_file_mktemp() created file
      to avoid a fd and inode leak when/if later passed to apr_file_setaside().
      [Yann Ylavic]
Index: mmap/unix/mmap.c
===================================================================
--- mmap/unix/mmap.c	(revision 1887062)
+++ mmap/unix/mmap.c	(working copy)
@@ -14,6 +14,8 @@
  * limitations under the License.
  */
 
+#include <assert.h>
+
 #include "apr.h"
 #include "apr_private.h"
 #include "apr_general.h"
@@ -33,6 +35,9 @@
 #if APR_HAVE_STDIO_H
 #include <stdio.h>
 #endif
+#if APR_HAVE_UNISTD_H
+#include <unistd.h>  /* for sysconf() */
+#endif
 #ifdef HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
@@ -42,6 +47,34 @@
 
 #if APR_HAS_MMAP || defined(BEOS)
 
+#ifndef BEOS
+struct mm_layout {
+    apr_mmap_t mm;
+    apr_off_t poffset;
+};
+
+static APR_INLINE
+apr_mmap_t *alloc_with_poffset(apr_pool_t *p)
+{
+    struct mm_layout *layout = apr_pcalloc(p, sizeof(*layout));
+    return &layout->mm;
+}
+
+static APR_INLINE
+void set_poffset(apr_mmap_t *mm, apr_off_t poffset)
+{
+    struct mm_layout *layout = (struct mm_layout *)mm;
+    layout->poffset = poffset;
+}
+
+static APR_INLINE
+apr_off_t get_poffset(apr_mmap_t *mm)
+{
+    struct mm_layout *layout = (struct mm_layout *)mm;
+    return layout->poffset;
+}
+#endif
+
 static apr_status_t mmap_cleanup(void *themmap)
 {
     apr_mmap_t *mm = themmap;
@@ -61,7 +94,7 @@ static apr_status_t mmap_cleanup(void *themmap)
 #ifdef BEOS
     rv = delete_area(mm->area);
 #else
-    rv = munmap(mm->mm, mm->size);
+    rv = munmap((char *)mm->mm - get_poffset(mm), mm->size + get_poffset(mm));
 #endif
     mm->mm = (void *)-1;
 
@@ -81,6 +114,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
     area_id aid = -1;
     uint32 pages = 0;
 #else
+    static long psize;
+    apr_off_t poffset = 0;
     apr_int32_t native_flags = 0;
 #endif
 
@@ -97,9 +132,11 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
     
     if (file == NULL || file->filedes == -1 || file->buffered)
         return APR_EBADF;
-    (*new) = (apr_mmap_t *)apr_pcalloc(cont, sizeof(apr_mmap_t));
-    
+
 #ifdef BEOS
+
+    *new = (apr_mmap_t *)apr_pcalloc(cont, sizeof(apr_mmap_t));
+
     /* XXX: mmap shouldn't really change the seek offset */
     apr_file_seek(file, APR_SET, &offset);
 
@@ -121,7 +158,9 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
         read(file->filedes, mm, size);
     
     (*new)->area = aid;
+
 #else
+    *new = alloc_with_poffset(cont);
 
     if (flag & APR_MMAP_WRITE) {
         native_flags |= PROT_WRITE;
@@ -130,8 +169,19 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
         native_flags |= PROT_READ;
     }
 
-    mm = mmap(NULL, size, native_flags, MAP_SHARED, file->filedes, offset);
+#if defined(_SC_PAGESIZE)
+    if (psize == 0) {
+        psize = sysconf(_SC_PAGESIZE);
+        /* the page size should be a power of two */
+        assert(psize > 0 && (psize & (psize - 1)) == 0);
+    }
+    poffset = offset & (apr_off_t)(psize - 1);
+#endif
 
+    mm = mmap(NULL, size + poffset,
+              native_flags, MAP_SHARED,
+              file->filedes, offset - poffset);
+
     if (mm == (void *)-1) {
         /* we failed to get an mmap'd file... */
         *new = NULL;
@@ -139,7 +189,8 @@ APR_DECLARE(apr_status_t) apr_mmap_create(apr_mmap
     }
 #endif
 
-    (*new)->mm = mm;
+    (*new)->mm = (char *)mm + poffset;
+    set_poffset(*new, poffset);
     (*new)->size = size;
     (*new)->cntxt = cont;
     APR_RING_ELEM_INIT(*new, link);
@@ -154,7 +205,12 @@ APR_DECLARE(apr_status_t) apr_mmap_dup(apr_mmap_t
                                        apr_mmap_t *old_mmap,
                                        apr_pool_t *p)
 {
+#ifdef BEOS
     *new_mmap = (apr_mmap_t *)apr_pmemdup(p, old_mmap, sizeof(apr_mmap_t));
+#else
+    *new_mmap = (apr_mmap_t *)apr_pmemdup(p, old_mmap,
+                                          sizeof(struct mm_layout));
+#endif
     (*new_mmap)->cntxt = p;
 
     APR_RING_INSERT_AFTER(old_mmap, *new_mmap, link);
Index: test/data/mmap_large_datafile.txt
===================================================================
Index: test/testmmap.c
===================================================================
--- test/testmmap.c	(revision 1887062)
+++ test/testmmap.c	(working copy)
@@ -35,7 +35,8 @@ static void not_implemented(abts_case *tc, void *d
 
 #else
 
-static char test_string[256]; /* read from the datafile */
+static apr_pool_t *ptest;
+static char *thisfdata; /* read from the datafile */
 static apr_mmap_t *themmap = NULL;
 static apr_file_t *thefile = NULL;
 static char *file1;
@@ -42,11 +43,22 @@ static char *file1;
 static apr_finfo_t thisfinfo;
 static apr_size_t thisfsize;
 
+static struct {
+    const char *filename;
+    apr_off_t offset;
+} test_set[] = {
+    { "/data/mmap_datafile.txt", 0 },
+    { "/data/mmap_large_datafile.txt", 65536 },
+    { "/data/mmap_large_datafile.txt", 66650 }, /* not page aligned */
+    { NULL, }
+};
+
 static void create_filename(abts_case *tc, void *data)
 {
+    const char *filename = data;
     char *oldfileptr;
 
-    apr_filepath_get(&file1, 0, p);
+    apr_filepath_get(&file1, 0, ptest);
 #ifndef NETWARE
 #ifdef WIN32
     ABTS_TRUE(tc, file1[1] == ':');
@@ -57,7 +69,7 @@ static void create_filename(abts_case *tc, void *d
     ABTS_TRUE(tc, file1[strlen(file1) - 1] != '/');
 
     oldfileptr = file1;
-    file1 = apr_pstrcat(p, file1,"/data/mmap_datafile.txt" ,NULL);
+    file1 = apr_pstrcat(ptest, file1, filename, NULL);
     ABTS_TRUE(tc, oldfileptr != file1);
 }
 
@@ -67,24 +79,15 @@ static void test_file_close(abts_case *tc, void *d
 
     rv = apr_file_close(thefile);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    thefile = NULL;
 }
    
-static void read_expected_contents(abts_case *tc, void *data)
-{
-    apr_status_t rv;
-    apr_size_t nbytes = sizeof(test_string) - 1;
-
-    rv = apr_file_read(thefile, test_string, &nbytes);
-    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
-    test_string[nbytes] = '\0';
-    thisfsize = strlen(test_string);
-}
-
 static void test_file_open(abts_case *tc, void *data)
 {
     apr_status_t rv;
 
-    rv = apr_file_open(&thefile, file1, APR_FOPEN_READ, APR_UREAD | APR_GREAD, p);
+    rv = apr_file_open(&thefile, file1, APR_FOPEN_READ,
+                       APR_UREAD | APR_GREAD, ptest);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
     ABTS_PTR_NOTNULL(tc, thefile);
 }
@@ -95,15 +98,42 @@ static void test_get_filesize(abts_case *tc, void
 
     rv = apr_file_info_get(&thisfinfo, APR_FINFO_NORM, thefile);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
-    ABTS_ASSERT(tc, "File size mismatch", thisfsize == thisfinfo.size);
+
+    thisfsize = thisfinfo.size;
+    thisfdata = apr_palloc(ptest, thisfsize + 1);
+    ABTS_PTR_NOTNULL(tc, thisfdata);
 }
 
+static void read_expected_contents(abts_case *tc, void *data)
+{
+    apr_off_t *offset = data;
+    apr_size_t nbytes = 0;
+    apr_status_t rv;
+
+    rv = apr_file_read_full(thefile, thisfdata, thisfsize, &nbytes);
+    ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    ABTS_ASSERT(tc, "File size mismatch", nbytes == thisfsize);
+    thisfdata[nbytes] = '\0';
+    ABTS_ASSERT(tc, "File content size mismatch",
+                strlen(thisfdata) == thisfsize);
+    ABTS_ASSERT(tc, "File size too small",
+                (apr_size_t)*offset < thisfsize);
+
+    /* From now, pretend that the file data and size don't include the
+     * offset, this avoids adding/substrating it to thisfdata/thisfsize
+     * all over the place in the next tests.
+     */
+    thisfdata += *offset;
+    thisfsize -= *offset;
+}
+
 static void test_mmap_create(abts_case *tc, void *data)
 {
+    apr_off_t *offset = data;
     apr_status_t rv;
 
-    rv = apr_mmap_create(&themmap, thefile, 0, (apr_size_t) thisfinfo.size, 
-		                 APR_MMAP_READ, p);
+    rv = apr_mmap_create(&themmap, thefile, *offset, thisfsize,
+                         APR_MMAP_READ, ptest);
     ABTS_PTR_NOTNULL(tc, themmap);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
 }
@@ -110,13 +140,12 @@ static void test_mmap_create(abts_case *tc, void *
 
 static void test_mmap_contents(abts_case *tc, void *data)
 {
-    
     ABTS_PTR_NOTNULL(tc, themmap);
     ABTS_PTR_NOTNULL(tc, themmap->mm);
     ABTS_SIZE_EQUAL(tc, thisfsize, themmap->size);
 
     /* Must use nEquals since the string is not guaranteed to be NULL terminated */
-    ABTS_STR_NEQUAL(tc, themmap->mm, test_string, thisfsize);
+    ABTS_STR_NEQUAL(tc, themmap->mm, thisfdata, thisfsize);
 }
 
 static void test_mmap_delete(abts_case *tc, void *data)
@@ -126,6 +155,7 @@ static void test_mmap_delete(abts_case *tc, void *
     ABTS_PTR_NOTNULL(tc, themmap);
     rv = apr_mmap_delete(themmap);
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
+    themmap = NULL;
 }
 
 static void test_mmap_offset(abts_case *tc, void *data)
@@ -138,8 +168,9 @@ static void test_mmap_offset(abts_case *tc, void *
 
     ABTS_INT_EQUAL(tc, APR_SUCCESS, rv);
     /* Must use nEquals since the string is not guaranteed to be NULL terminated */
-    ABTS_STR_NEQUAL(tc, addr, test_string + 5, thisfsize-5);
+    ABTS_STR_NEQUAL(tc, addr, thisfdata + 5, thisfsize - 5);
 }
+
 #endif
 
 abts_suite *testmmap(abts_suite *suite)
@@ -147,15 +178,21 @@ abts_suite *testmmap(abts_suite *suite)
     suite = ADD_SUITE(suite)
 
 #if APR_HAS_MMAP    
-    abts_run_test(suite, create_filename, NULL);
-    abts_run_test(suite, test_file_open, NULL);
-    abts_run_test(suite, read_expected_contents, NULL);
-    abts_run_test(suite, test_get_filesize, NULL);
-    abts_run_test(suite, test_mmap_create, NULL);
-    abts_run_test(suite, test_mmap_contents, NULL);
-    abts_run_test(suite, test_mmap_offset, NULL);
-    abts_run_test(suite, test_mmap_delete, NULL);
-    abts_run_test(suite, test_file_close, NULL);
+    apr_size_t i;
+    apr_pool_create(&ptest, p);
+    for (i = 0; test_set[i].filename; ++i) {
+        abts_run_test(suite, create_filename, (void *)test_set[i].filename);
+        abts_run_test(suite, test_file_open, NULL);
+        abts_run_test(suite, test_get_filesize, NULL);
+        abts_run_test(suite, read_expected_contents, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_create, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_contents, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_offset, &test_set[i].offset);
+        abts_run_test(suite, test_mmap_delete, NULL);
+        abts_run_test(suite, test_file_close, NULL);
+        apr_pool_clear(ptest);
+    }
+    apr_pool_destroy(ptest);
 #else
     abts_run_test(suite, not_implemented, NULL);
 #endif
Index: .
===================================================================
--- .	(revision 1887062)
+++ .	(working copy)

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
## -0,0 +0,1 ##
   Merged /apr/apr/trunk:r1887060


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic