You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Yann Ylavic <yl...@gmail.com> on 2021/03/02 00:54:37 UTC
Re: svn commit: r1887060 - in /apr/apr/trunk: CHANGES
include/apr_mmap.h mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testmmap.c
On Tue, Mar 2, 2021 at 1:37 AM <yl...@apache.org> wrote:
>
> Author: ylavic
> Date: Tue Mar 2 00:37:18 2021
> New Revision: 1887060
>
> URL: http://svn.apache.org/viewvc?rev=1887060&view=rev
> Log:
> Align apr_mmap()ing offset to a page boundary. PR 65158.
[]
> --- apr/apr/trunk/include/apr_mmap.h (original)
> +++ apr/apr/trunk/include/apr_mmap.h Tue Mar 2 00:37:18 2021
> @@ -62,11 +62,10 @@ typedef struct apr_mmap_t apr
> struct apr_mmap_t {
> /** The pool the mmap structure was allocated out of. */
> apr_pool_t *cntxt;
> -#ifdef BEOS
> +#if defined(BEOS)
> /** An area ID. Only valid on BeOS */
> area_id area;
> -#endif
> -#ifdef WIN32
> +#elif defined(WIN32)
> /** The handle of the file mapping */
> HANDLE mhandle;
> /** The start of the real memory page area (mapped view) */
> @@ -75,6 +74,8 @@ struct apr_mmap_t {
> apr_off_t pstart;
> apr_size_t psize;
> apr_off_t poffset;
> +#else
> + apr_off_t poffset;
> #endif
> /** The start of the memory mapped area */
> void *mm;
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)?
Regards;
Yann.
Re: svn commit: r1887060 - in /apr/apr/trunk: CHANGES
include/apr_mmap.h mmap/unix/mmap.c test/data/mmap_large_datafile.txt test/testmmap.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Mar 2, 2021 at 8:35 AM Ruediger Pluem <rp...@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.
Re: svn commit: r1887060 - in /apr/apr/trunk: CHANGES
include/apr_mmap.h mmap/unix/mmap.c test/data/mmap_large_datafile.txt
test/testmmap.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 3/2/21 1:54 AM, Yann Ylavic wrote:
> On Tue, Mar 2, 2021 at 1:37 AM <yl...@apache.org> wrote:
>>
>> Author: ylavic
>> Date: Tue Mar 2 00:37:18 2021
>> New Revision: 1887060
>>
>> URL: http://svn.apache.org/viewvc?rev=1887060&view=rev
>> Log:
>> Align apr_mmap()ing offset to a page boundary. PR 65158.
> []
>> --- apr/apr/trunk/include/apr_mmap.h (original)
>> +++ apr/apr/trunk/include/apr_mmap.h Tue Mar 2 00:37:18 2021
>> @@ -62,11 +62,10 @@ typedef struct apr_mmap_t apr
>> struct apr_mmap_t {
>> /** The pool the mmap structure was allocated out of. */
>> apr_pool_t *cntxt;
>> -#ifdef BEOS
>> +#if defined(BEOS)
>> /** An area ID. Only valid on BeOS */
>> area_id area;
>> -#endif
>> -#ifdef WIN32
>> +#elif defined(WIN32)
>> /** The handle of the file mapping */
>> HANDLE mhandle;
>> /** The start of the real memory page area (mapped view) */
>> @@ -75,6 +74,8 @@ struct apr_mmap_t {
>> apr_off_t pstart;
>> apr_size_t psize;
>> apr_off_t poffset;
>> +#else
>> + apr_off_t poffset;
>> #endif
>> /** The start of the memory mapped area */
>> void *mm;
>
> 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.
Regards
Rüdiger