You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@locus.apache.org on 2000/12/07 07:59:00 UTC

cvs commit: apr/mmap/win32 mmap.c

wrowe       00/12/06 22:58:59

  Modified:    .        CHANGES aprlib.dsp
               include  apr.hw apr_mmap.h
  Added:       mmap/win32 mmap.c
  Log:
    Implement Win32 MMAP support.
  
  Revision  Changes    Path
  1.17      +3 -0      apr/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/apr/CHANGES,v
  retrieving revision 1.16
  retrieving revision 1.17
  diff -u -r1.16 -r1.17
  --- CHANGES	2000/12/07 05:00:27	1.16
  +++ CHANGES	2000/12/07 06:58:57	1.17
  @@ -1,4 +1,7 @@
   Changes with APR a9
  +
  +  *) Add Win32 MMAP support [William Rowe]
  +
     *) Allow the APR programmer to specify if the MMAP is read-only or
        write-able.
        [Ryan Bloom and Will Rowe]
  
  
  
  1.51      +13 -2     apr/aprlib.dsp
  
  Index: aprlib.dsp
  ===================================================================
  RCS file: /home/cvs/apr/aprlib.dsp,v
  retrieving revision 1.50
  retrieving revision 1.51
  diff -u -r1.50 -r1.51
  --- aprlib.dsp	2000/12/05 22:58:31	1.50
  +++ aprlib.dsp	2000/12/07 06:58:57	1.51
  @@ -68,9 +68,8 @@
   RSC=rc.exe
   # ADD BASE RSC /l 0x409
   # ADD RSC /l 0x409
  -# ADD BASE CPP /nologo /MTd /W3 /GX /ZI /Od /D "WIN32" /D "_DEBUG" /D "_WINDOWS" /YX /FD /c
  +# ADD BASE CPP /nologo /MTd /W3 /GX /Od /D "WIN32" /D "_DEBUG" /D "_WINDOWS" /YX /FD /ZI /c
   # ADD CPP /nologo /MDd /W3 /GX /ZI /Od /I "./include" /I "./include/arch" /I "./include/arch/win32" /I "./include/arch/unix" /D "_DEBUG" /D "APR_DECLARE_EXPORT" /D "WIN32" /D "_WINDOWS" /FD /c
  -# SUBTRACT CPP /YX
   BSC32=bscmake.exe
   # ADD BASE BSC32 /nologo
   # ADD BSC32 /nologo
  @@ -369,6 +368,18 @@
   
   SOURCE=.\shmem\win32\shmem.c
   # PROP Exclude_From_Build 1
  +# End Source File
  +# End Group
  +# Begin Group "mmap"
  +
  +# PROP Default_Filter ""
  +# Begin Source File
  +
  +SOURCE=.\mmap\unix\common.c
  +# End Source File
  +# Begin Source File
  +
  +SOURCE=.\mmap\win32\mmap.c
   # End Source File
   # End Group
   # End Group
  
  
  
  1.41      +1 -1      apr/include/apr.hw
  
  Index: apr.hw
  ===================================================================
  RCS file: /home/cvs/apr/include/apr.hw,v
  retrieving revision 1.40
  retrieving revision 1.41
  diff -u -r1.40 -r1.41
  --- apr.hw	2000/12/05 00:14:59	1.40
  +++ apr.hw	2000/12/07 06:58:58	1.41
  @@ -171,7 +171,7 @@
   #define APR_HAS_SHARED_MEMORY     0
   #define APR_HAS_THREADS           1
   #define APR_HAS_SENDFILE          1
  -#define APR_HAS_MMAP              0
  +#define APR_HAS_MMAP              1
   #define APR_HAS_FORK              0
   #define APR_HAS_RANDOM            1
   #define APR_HAS_XLATE             0
  
  
  
  1.19      +10 -0     apr/include/apr_mmap.h
  
  Index: apr_mmap.h
  ===================================================================
  RCS file: /home/cvs/apr/include/apr_mmap.h,v
  retrieving revision 1.18
  retrieving revision 1.19
  diff -u -r1.18 -r1.19
  --- apr_mmap.h	2000/12/07 05:00:27	1.18
  +++ apr_mmap.h	2000/12/07 06:58:59	1.19
  @@ -90,6 +90,16 @@
       /** An area ID.  Only valid on BeOS */
       area_id area;
   #endif
  +#ifdef WIN32
  +    /** The handle of the file mapping */
  +    HANDLE mhandle;
  +    /** The start of the real memory page area (mapped view) */
  +    void *mv;
  +    /** The physical start, size and offset */
  +    size_t pstart;
  +    size_t psize;
  +    size_t poffset;
  +#endif
       /** The start of the memory mapped area */
       void *mm;
       /** The amount of data in the mmap */
  
  
  
  1.1                  apr/mmap/win32/mmap.c
  
  Index: mmap.c
  ===================================================================
  /* ====================================================================
   * The Apache Software License, Version 1.1
   *
   * Copyright (c) 2000 The Apache Software Foundation.  All rights
   * reserved.
   *
   * Redistribution and use in source and binary forms, with or without
   * modification, are permitted provided that the following conditions
   * are met:
   *
   * 1. Redistributions of source code must retain the above copyright
   *    notice, this list of conditions and the following disclaimer.
   *
   * 2. Redistributions in binary form must reproduce the above copyright
   *    notice, this list of conditions and the following disclaimer in
   *    the documentation and/or other materials provided with the
   *    distribution.
   *
   * 3. The end-user documentation included with the redistribution,
   *    if any, must include the following acknowledgment:
   *       "This product includes software developed by the
   *        Apache Software Foundation (http://www.apache.org/)."
   *    Alternately, this acknowledgment may appear in the software itself,
   *    if and wherever such third-party acknowledgments normally appear.
   *
   * 4. The names "Apache" and "Apache Software Foundation" must
   *    not be used to endorse or promote products derived from this
   *    software without prior written permission. For written
   *    permission, please contact apache@apache.org.
   *
   * 5. Products derived from this software may not be called "Apache",
   *    nor may "Apache" appear in their name, without prior written
   *    permission of the Apache Software Foundation.
   *
   * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
   * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
   * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
   * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
   * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
   * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
   * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
   * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
   * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
   * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
   * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
   * SUCH DAMAGE.
   * ====================================================================
   *
   * This software consists of voluntary contributions made by many
   * individuals on behalf of the Apache Software Foundation.  For more
   * information on the Apache Software Foundation, please see
   * <http://www.apache.org/>.
   */
  
  #include "apr.h"
  #include "apr_private.h"
  #include "apr_general.h"
  #include "apr_mmap.h"
  #include "apr_errno.h"
  #include "fileio.h"
  #include "apr_portable.h"
  
  #if APR_HAS_MMAP
  
  static apr_status_t mmap_cleanup(void *themmap)
  {
      apr_mmap_t *mm = themmap;
      apr_status_t rv = 0;
      if (mm->mv) {
          if (!UnmapViewOfFile(mm->mv))
          {
              apr_status_t rv = apr_get_os_error();
              CloseHandle(mm->mhandle);
              mm->mv = NULL;
              mm->mhandle = NULL;
              return rv;
          }
          mm->mv = NULL;
      }
      if (mm->mhandle) 
      {
          if (!CloseHandle(mm->mhandle))
          {
              apr_status_t rv = apr_get_os_error();
              CloseHandle(mm->mhandle);
              mm->mhandle = NULL;
              return rv;
          }
          mm->mhandle = NULL;
      }
      return APR_SUCCESS;
  }
  
  apr_status_t apr_mmap_create(apr_mmap_t **new, apr_file_t *file, apr_off_t offset, 
                               apr_size_t size, apr_int32_t flag, apr_pool_t *cont)
  {
      static DWORD memblock = 0;
      DWORD fmaccess = 0, mvaccess = 0;
  
      if (flag & APR_MMAP_WRITE)
          fmaccess |= PAGE_READWRITE;
      else if (flag & APR_MMAP_READ)
          fmaccess |= PAGE_READONLY;
  
      if (flag & APR_MMAP_READ)
          mvaccess |= FILE_MAP_READ;
      if (flag & APR_MMAP_WRITE)
          mvaccess |= FILE_MAP_WRITE;
  
      if (!file || !file->filehand || file->filehand == INVALID_HANDLE_VALUE
          || file->buffered)
          return APR_EBADF;
  
      if (!memblock)
      {
          SYSTEM_INFO si;
          GetSystemInfo(&si);
          memblock = si.dwAllocationGranularity;
      }   
      
      *new = apr_pcalloc(cont, sizeof(apr_mmap_t));
      (*new)->pstart = (offset / memblock) * memblock;
      (*new)->psize = (offset % memblock) + size;
      (*new)->poffset = offset - (*new)->pstart;
  
      (*new)->mhandle = CreateFileMapping(file->filehand, NULL, fmaccess,
                                          0, (*new)->psize, NULL);
      if (!(*new)->mhandle || (*new)->mhandle == INVALID_HANDLE_VALUE)
      {
          *new = NULL;
          return apr_get_os_error();
      }
  
      (*new)->mv = MapViewOfFile((*new)->mhandle, mvaccess, 0,
                                 (*new)->poffset, (*new)->psize);
      if (!(*new)->mv)
      {
          apr_status_t rv = apr_get_os_error();
          CloseHandle((*new)->mhandle);
          *new = NULL;
          return rv;
      }
  
      (*new)->mm = (char*)((*new)->mv) + offset;
      (*new)->size = size;
      (*new)->cntxt = cont;
  
      /* register the cleanup... */
      apr_register_cleanup((*new)->cntxt, (void*)(*new), mmap_cleanup,
                           apr_null_cleanup);
      return APR_SUCCESS;
  }
  
  apr_status_t apr_mmap_delete(apr_mmap_t *mmap)
  {
      apr_status_t rv;
  
      if ((rv = mmap_cleanup(mmap)) == APR_SUCCESS) {
          apr_kill_cleanup(mmap->cntxt, mmap, mmap_cleanup);
          return APR_SUCCESS;
      }
      return rv;
  }
  
  #endif
  
  
  

RE: cvs commit: apr/mmap/win32 mmap.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Thursday, December 07, 2000 7:52 AM
> 
> On Thu, Dec 07, 2000 at 07:33:31AM -0600, William A. Rowe, Jr. wrote:
> > rbb and I agreed last night,
> 
> Agreed *what* ?

with you, that apr_mmap_t aught to go incomplete :-)

> > except that there are some optimizations
> > that go away with hiding the type
> 
> What kind of optimizations? Are we talking a few cycles? 
> Let's go with the hiding and take the hit of a few cycles.

I don't have a huge issue with it.

> > We left it for you to find :-)
> 
> Find what? I'm lost.

the (inappropriately) complete apr_mmap_t type.

Making it incomplete needed to be it's own patch, anyways.

> Doing the [#if APR_HAS_MMAP in win32/mmap.c] is confusing because 
> (as I mentioned) it seems to imply that APR_HAS_MMAP might be zero 
> in some (unknown) situation.

And maybe it will be ... when I've build and tested on a 9x platform.
Don't know yet that it will work.  I'm not saying _I_ want to make my
inactive, I'm saying if others [Alan, Bill, Jeff, whomever] find a
quirck, they can work around it.  That's how I coded APR_HAS_UNICODE_FS,
since it's a radically new experiment [which you and I still need to
hammer out, but please save discussion for the afternoon :-]

> Leave a comment rather than code structure.
> 
> /* ### this hasn't been tested on all Win32 systems */
> That is a LOT clearer than "#if APR_HAS_MMAP". The #if says nothing about
> testing on other systems. If that was your intent, then say so. :-)

On this sort of thing, fine.  I'll apply a bit later.  {I'm leaving alone
the 'other' discussion for this morning, too much on my plate.  Patch will
preceed the canonfilename Apache stuff.)

Re: cvs commit: apr/mmap/win32 mmap.c

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Dec 07, 2000 at 07:33:31AM -0600, William A. Rowe, Jr. wrote:
> > From: Greg Stein [mailto:gstein@lyra.org]
> > Sent: Thursday, December 07, 2000 2:09 AM
>...
> > Non-win32-specific question: why is the MMAP structure 
> > visible? Shouldn't that be an opaque structure?

Dude. You're being obscure below...

> rbb and I agreed last night,

Agreed *what* ?

> except that there are some optimizations
> that go away with hiding the type

What kind of optimizations? Are we talking a few cycles? Let's go with the
hiding and take the hit of a few cycles.

> (contrawise, common.c code should be
> macros, if this type remains visible.)  We left it for you to find :-)

Find what? I'm lost.

> > >...
> > >   #include "apr.h"
> > >   #include "apr_private.h"
> > >   #include "apr_general.h"
> > >   #include "apr_mmap.h"
> > >   #include "apr_errno.h"
> > >   #include "fileio.h"
> > >   #include "apr_portable.h"
> > >   
> > >   #if APR_HAS_MMAP
> > 
> > Why put this in here? apr_private.hw defines it to 1. The above statement
> > seems to imply that sometimes it will be non-zero. And that just isn't the
> > case...
> 
> Because I always leave the option to -quickly- yank new code.

Add a #if 0 if you want to yank it. Check in a reverse patch. Delete the
file. Whatever.

Doing the above is confusing because (as I mentioned) it seems to imply that
APR_HAS_MMAP might be zero in some (unknown) situation.

> The new
> filename canonical will go into the server first before Sunday night, and
> will retain (for now) a bunch of extra cruft in the non-canonical compilation
> path.

1) post a patch first. I know there is still a good amount of "what the
   hell" going on, even after the hackathon.

2) screw leaving the cruft. we can always reverse patch. leaving the old
   code will just confuse matters, yet we'll be trying to review the new
   stuff.

Point is (same as above): make CVS work for you... don't add complexity to
the code to replicate what CVS can already do. Don't add "#if APR_HAS_MMAP"
on the *chance* that you might want to back it out; do something if/when you
need to back it.

> This test could go away whenever.  The other reason it's left is to remember 
> that it isn't tested on all win platforms, and we know that the Win32 API
> discrepancies are worse than five releases of three different ports of Unix.

Leave a comment rather than code structure.

/* ### this hasn't been tested on all Win32 systems */

That is a LOT clearer than "#if APR_HAS_MMAP". The #if says nothing about
testing on other systems. If that was your intent, then say so. :-)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: cvs commit: apr/mmap/win32 mmap.c

Posted by rb...@covalent.net.
> >   +#ifdef WIN32
> >   +    /** The handle of the file mapping */
> >   +    HANDLE mhandle;
> >   +    /** The start of the real memory page area (mapped view) */
> >   +    void *mv;
> >   +    /** The physical start, size and offset */
> >   +    size_t pstart;
> >   +    size_t psize;
> >   +    size_t poffset;
> >   +#endif
> 
> Non-win32-specific question: why is the MMAP structure visible? Shouldn't
> that be an opaque structure?

It was to begin with, then I started adding MMAP support to 2.0, and
making it a visible structure made adding it to Apache much easier, and it
just made sense.  Since I added it, the code has changed, and we won't
have any problems hiding this structure again.  Will and I discussed this
last night, and I'll probably end up making this structure incomplete
today if I have a chance.

Ryan
_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------


RE: cvs commit: apr/mmap/win32 mmap.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: Thursday, December 07, 2000 2:09 AM
> 
> > wrowe       00/12/06 22:58:59
> > 
> >   --- apr_mmap.h	2000/12/07 05:00:27	1.18
> >   +++ apr_mmap.h	2000/12/07 06:58:59	1.19
> >   @@ -90,6 +90,16 @@
> >        /** An area ID.  Only valid on BeOS */
> >        area_id area;
> >    #endif
> >   +#ifdef WIN32
> >   +    /** The handle of the file mapping */
> >   +    HANDLE mhandle;
> >   +    /** The start of the real memory page area (mapped view) */
> >   +    void *mv;
> >   +    /** The physical start, size and offset */
> >   +    size_t pstart;
> >   +    size_t psize;
> >   +    size_t poffset;
> >   +#endif
> 
> Non-win32-specific question: why is the MMAP structure 
> visible? Shouldn't that be an opaque structure?

rbb and I agreed last night, except that there are some optimizations
that go away with hiding the type (contrawise, common.c code should be
macros, if this type remains visible.)  We left it for you to find :-)

> >...
> >   #include "apr.h"
> >   #include "apr_private.h"
> >   #include "apr_general.h"
> >   #include "apr_mmap.h"
> >   #include "apr_errno.h"
> >   #include "fileio.h"
> >   #include "apr_portable.h"
> >   
> >   #if APR_HAS_MMAP
> 
> Why put this in here? apr_private.hw defines it to 1. The above statement
> seems to imply that sometimes it will be non-zero. And that just isn't the
> case...

Because I always leave the option to -quickly- yank new code.  The new
filename canonical will go into the server first before Sunday night, and
will retain (for now) a bunch of extra cruft in the non-canonical compilation
path.  

This test could go away whenever.  The other reason it's left is to remember 
that it isn't tested on all win platforms, and we know that the Win32 API
discrepancies are worse than five releases of three different ports of Unix.


Re: cvs commit: apr/mmap/win32 mmap.c

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Dec 07, 2000 at 06:59:00AM -0000, wrowe@locus.apache.org wrote:
> wrowe       00/12/06 22:58:59
> 
>   Modified:    .        CHANGES aprlib.dsp
>                include  apr.hw apr_mmap.h
>   Added:       mmap/win32 mmap.c
>   Log:
>     Implement Win32 MMAP support.
>...
>   --- apr_mmap.h	2000/12/07 05:00:27	1.18
>   +++ apr_mmap.h	2000/12/07 06:58:59	1.19
>   @@ -90,6 +90,16 @@
>        /** An area ID.  Only valid on BeOS */
>        area_id area;
>    #endif
>   +#ifdef WIN32
>   +    /** The handle of the file mapping */
>   +    HANDLE mhandle;
>   +    /** The start of the real memory page area (mapped view) */
>   +    void *mv;
>   +    /** The physical start, size and offset */
>   +    size_t pstart;
>   +    size_t psize;
>   +    size_t poffset;
>   +#endif

Non-win32-specific question: why is the MMAP structure visible? Shouldn't
that be an opaque structure?

>...
>   #include "apr.h"
>   #include "apr_private.h"
>   #include "apr_general.h"
>   #include "apr_mmap.h"
>   #include "apr_errno.h"
>   #include "fileio.h"
>   #include "apr_portable.h"
>   
>   #if APR_HAS_MMAP

Why put this in here? apr_private.hw defines it to 1. The above statement
seems to imply that sometimes it will be non-zero. And that just isn't the
case...

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/