You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by yl...@apache.org on 2021/03/11 16:21:07 UTC
svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES
mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c
test/testmmap.c
Author: ylavic
Date: Thu Mar 11 16:21:07 2021
New Revision: 1887497
URL: http://svn.apache.org/viewvc?rev=1887497&view=rev
Log:
Merge r1887060 from trunk:
Align apr_mmap()ing offset to a page boundary. PR 65158.
This is requirement for unix systems, and otherwise can cause failures or
undefined behaviour with some filesystems.
Add apr_mmap_t->poffset on unixes to track the offset to the previous page,
relative to the user requested/actual offset. This allows ->mm and ->size to
still point to the actual data, while munmap() now deletes the full range
from the start of the page.
Tests updated accordingly.
* Changes between r1887060 and this 1.7.x backport
Since the layout of struct apr_mmap_t cannot change (non opaque), the poffset
is over-allocated in mmap/unix/mmap.cmmap.c which uses this layout internally:
struct mm_layout {
apr_mmap_t mm;
apr_off_t poffset;
};
This works for all the apr_mmap_t created by apr_mmap_create() with which any
apr_mmap_*() function can then be called.
If a user creates an apr_mmap_t "by hand", with this change it cannot be passed
to apr_mmap_dup() without causing an out-of-bound read. This is hardly a new
limitation though, the refcounting which is the point of apr_mmap_dup() and
apr_mmap_delete() comes from the private function mmap_cleanup() anyway, so
there is no point in using these two functions with hand made apr_mmap_t (if
there ever was a point in the first place to create an hand made apr_mmap_t..).
Submitted by: ylavic
Added:
apr/apr/branches/1.7.x/test/data/mmap_large_datafile.txt
- copied unchanged from r1887060, apr/apr/trunk/test/data/mmap_large_datafile.txt
Modified:
apr/apr/branches/1.7.x/ (props changed)
apr/apr/branches/1.7.x/CHANGES
apr/apr/branches/1.7.x/mmap/unix/mmap.c
apr/apr/branches/1.7.x/test/testfnmatch.c
apr/apr/branches/1.7.x/test/testmmap.c
Propchange: apr/apr/branches/1.7.x/
------------------------------------------------------------------------------
Merged /apr/apr/trunk:r1887060
Modified: apr/apr/branches/1.7.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?rev=1887497&r1=1887496&r2=1887497&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/CHANGES [utf-8] (original)
+++ apr/apr/branches/1.7.x/CHANGES [utf-8] Thu Mar 11 16:21:07 2021
@@ -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]
Modified: apr/apr/branches/1.7.x/mmap/unix/mmap.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/mmap/unix/mmap.c?rev=1887497&r1=1887496&r2=1887497&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/mmap/unix/mmap.c (original)
+++ apr/apr/branches/1.7.x/mmap/unix/mmap.c Thu Mar 11 16:21:07 2021
@@ -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 *t
#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_creat
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_creat
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_creat
read(file->filedes, mm, size);
(*new)->area = aid;
+
#else
+ *new = alloc_with_poffset(cont);
if (flag & APR_MMAP_WRITE) {
native_flags |= PROT_WRITE;
@@ -130,7 +169,18 @@ APR_DECLARE(apr_status_t) apr_mmap_creat
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... */
@@ -139,7 +189,8 @@ APR_DECLARE(apr_status_t) apr_mmap_creat
}
#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(a
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);
Modified: apr/apr/branches/1.7.x/test/testfnmatch.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/test/testfnmatch.c?rev=1887497&r1=1887496&r2=1887497&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/test/testfnmatch.c (original)
+++ apr/apr/branches/1.7.x/test/testfnmatch.c Thu Mar 11 16:21:07 2021
@@ -23,7 +23,7 @@
* .txt extension in the data directory at the time testfnmatch
* happens to be run (!?!). */
-#define NUM_FILES (5)
+#define NUM_FILES (6)
#define APR_FNM_BITS 15
#define APR_FNM_FAILBIT 256
Modified: apr/apr/branches/1.7.x/test/testmmap.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/test/testmmap.c?rev=1887497&r1=1887496&r2=1887497&view=diff
==============================================================================
--- apr/apr/branches/1.7.x/test/testmmap.c (original)
+++ apr/apr/branches/1.7.x/test/testmmap.c Thu Mar 11 16:21:07 2021
@@ -35,18 +35,30 @@ static void not_implemented(abts_case *t
#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;
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 *t
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 *t
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,28 +98,54 @@ static void test_get_filesize(abts_case
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);
}
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 *
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 *
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
Re: svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c test/testmmap.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
IMNSO opinion, this seems like a somewhat safe 1.8 change. What you
want to ask is how someone who "discovers" apr.so.1 (.8) would react
with 1.7 code, and if that's clean and using only public API's, it
makes sense to me. That's not the case for other changes, but for this
one it shouldn't shock anyone who doesn't explicitly ask for -lapr
against libapr.so.1.7
On Thu, Jun 30, 2022 at 7:39 AM Yann Ylavic <yl...@gmail.com> wrote:
>
> On Wed, Jun 29, 2022 at 4:49 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
> >
> > The eventual change still is present for apr
> > 1.8 adopters.
> >
> > Thoughts? ylavic are you willing to revert?
>
> It seems that no one is using apr_mmap_create() with an offset != 0 or
> the bug would have been reported (PR 65158 proved to be a cifs
> filesystem bug finally). Well, apr_bucket_file reads can use an mmap
> offset if planets misalign still, and that's possibly why we had quite
> some issues for "EnableMMap on" in httpd (IIRC), but since it was
> never nailed to this bug it usually ends with "please EnableMMap =>
> off"..
> So let's revert, if something comes up around this, it's probably
> another case where I'll first ask: can you reproduce with apr-1.8?
>
> For 1.8.x then (we can fix it there right?), I'd rather we revert this
> change and r1887508 to merge trunk's r1887060 and r1887506 directly
> (i.e. without the API workaround-try of this 1.7.x merge).
> That is, add the "poffset" field to (the end of) apr_mmap_t and be
> done with no tricks. But since I'm not sure what we can or not do with
> a minor bump I'll ask, can we do that?
>
> Regards;
> Yann.
Re: svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c test/testmmap.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Jun 29, 2022 at 4:49 PM William A Rowe Jr <wr...@rowe-clan.net> wrote:
>
> The eventual change still is present for apr
> 1.8 adopters.
>
> Thoughts? ylavic are you willing to revert?
It seems that no one is using apr_mmap_create() with an offset != 0 or
the bug would have been reported (PR 65158 proved to be a cifs
filesystem bug finally). Well, apr_bucket_file reads can use an mmap
offset if planets misalign still, and that's possibly why we had quite
some issues for "EnableMMap on" in httpd (IIRC), but since it was
never nailed to this bug it usually ends with "please EnableMMap =>
off"..
So let's revert, if something comes up around this, it's probably
another case where I'll first ask: can you reproduce with apr-1.8?
For 1.8.x then (we can fix it there right?), I'd rather we revert this
change and r1887508 to merge trunk's r1887060 and r1887506 directly
(i.e. without the API workaround-try of this 1.7.x merge).
That is, add the "poffset" field to (the end of) apr_mmap_t and be
done with no tricks. But since I'm not sure what we can or not do with
a minor bump I'll ask, can we do that?
Regards;
Yann.
Re: svn commit: r1887497 - in /apr/apr/branches/1.7.x: ./ CHANGES mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testfnmatch.c test/testmmap.c
Posted by William A Rowe Jr <wr...@rowe-clan.net>.
This change does introduce an API change, and a somewhat possibly dangerous
structure size alteration, and seems to be out of bounds for apr 1.7.x
branch, as
identified in the commit message.
Anyone using a build crossing runtimes between apr 1.7.0 and 1.7.1 may
experience
unintended side-effects. The eventual change still is present for apr
1.8 adopters.
Thoughts? ylavic are you willing to revert?
On Thu, Mar 11, 2021 at 10:21 AM <yl...@apache.org> wrote:
>
> Author: ylavic
> Date: Thu Mar 11 16:21:07 2021
> New Revision: 1887497
>
> URL: http://svn.apache.org/viewvc?rev=1887497&view=rev
> Log:
> Merge r1887060 from trunk:
>
> Align apr_mmap()ing offset to a page boundary. PR 65158.
>
> This is requirement for unix systems, and otherwise can cause failures or
> undefined behaviour with some filesystems.
>
> Add apr_mmap_t->poffset on unixes to track the offset to the previous page,
> relative to the user requested/actual offset. This allows ->mm and ->size to
> still point to the actual data, while munmap() now deletes the full range
> from the start of the page.
>
> Tests updated accordingly.
>
>
> * Changes between r1887060 and this 1.7.x backport
>
> Since the layout of struct apr_mmap_t cannot change (non opaque), the poffset
> is over-allocated in mmap/unix/mmap.cmmap.c which uses this layout internally:
> struct mm_layout {
> apr_mmap_t mm;
> apr_off_t poffset;
> };
>
> This works for all the apr_mmap_t created by apr_mmap_create() with which any
> apr_mmap_*() function can then be called.
>
> If a user creates an apr_mmap_t "by hand", with this change it cannot be passed
> to apr_mmap_dup() without causing an out-of-bound read. This is hardly a new
> limitation though, the refcounting which is the point of apr_mmap_dup() and
> apr_mmap_delete() comes from the private function mmap_cleanup() anyway, so
> there is no point in using these two functions with hand made apr_mmap_t (if
> there ever was a point in the first place to create an hand made apr_mmap_t..).
>
>
> Submitted by: ylavic
>
> Added:
> apr/apr/branches/1.7.x/test/data/mmap_large_datafile.txt
> - copied unchanged from r1887060, apr/apr/trunk/test/data/mmap_large_datafile.txt
> Modified:
> apr/apr/branches/1.7.x/ (props changed)
> apr/apr/branches/1.7.x/CHANGES
> apr/apr/branches/1.7.x/mmap/unix/mmap.c
> apr/apr/branches/1.7.x/test/testfnmatch.c
> apr/apr/branches/1.7.x/test/testmmap.c
>
> Propchange: apr/apr/branches/1.7.x/
> ------------------------------------------------------------------------------
> Merged /apr/apr/trunk:r1887060
>
> Modified: apr/apr/branches/1.7.x/CHANGES
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/CHANGES?rev=1887497&r1=1887496&r2=1887497&view=diff
> ==============================================================================
> --- apr/apr/branches/1.7.x/CHANGES [utf-8] (original)
> +++ apr/apr/branches/1.7.x/CHANGES [utf-8] Thu Mar 11 16:21:07 2021
> @@ -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]
>
> Modified: apr/apr/branches/1.7.x/mmap/unix/mmap.c
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/mmap/unix/mmap.c?rev=1887497&r1=1887496&r2=1887497&view=diff
> ==============================================================================
> --- apr/apr/branches/1.7.x/mmap/unix/mmap.c (original)
> +++ apr/apr/branches/1.7.x/mmap/unix/mmap.c Thu Mar 11 16:21:07 2021
> @@ -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 *t
> #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_creat
> 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_creat
>
> 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_creat
> read(file->filedes, mm, size);
>
> (*new)->area = aid;
> +
> #else
> + *new = alloc_with_poffset(cont);
>
> if (flag & APR_MMAP_WRITE) {
> native_flags |= PROT_WRITE;
> @@ -130,7 +169,18 @@ APR_DECLARE(apr_status_t) apr_mmap_creat
> 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... */
> @@ -139,7 +189,8 @@ APR_DECLARE(apr_status_t) apr_mmap_creat
> }
> #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(a
> 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);
>
> Modified: apr/apr/branches/1.7.x/test/testfnmatch.c
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/test/testfnmatch.c?rev=1887497&r1=1887496&r2=1887497&view=diff
> ==============================================================================
> --- apr/apr/branches/1.7.x/test/testfnmatch.c (original)
> +++ apr/apr/branches/1.7.x/test/testfnmatch.c Thu Mar 11 16:21:07 2021
> @@ -23,7 +23,7 @@
> * .txt extension in the data directory at the time testfnmatch
> * happens to be run (!?!). */
>
> -#define NUM_FILES (5)
> +#define NUM_FILES (6)
>
> #define APR_FNM_BITS 15
> #define APR_FNM_FAILBIT 256
>
> Modified: apr/apr/branches/1.7.x/test/testmmap.c
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.7.x/test/testmmap.c?rev=1887497&r1=1887496&r2=1887497&view=diff
> ==============================================================================
> --- apr/apr/branches/1.7.x/test/testmmap.c (original)
> +++ apr/apr/branches/1.7.x/test/testmmap.c Thu Mar 11 16:21:07 2021
> @@ -35,18 +35,30 @@ static void not_implemented(abts_case *t
>
> #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;
> 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 *t
> 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 *t
>
> 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,28 +98,54 @@ static void test_get_filesize(abts_case
>
> 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);
> }
>
> 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 *
> 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 *
>
> 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
>
>