You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <jw...@virginia.edu> on 2002/11/21 00:12:15 UTC

apr_mmap_t

Does anybody have any idea at all why apr_mmap_t is not an incomplete
type?  I have to change its size and that's a bit of a PITA because, while
it SHOULD not, it COULD break binary compat for stupid modules who use
sizeof(apr_mmap_t) for something.

--Cliff


Re: apr_mmap_t

Posted by rb...@apache.org.

On Sat, 23 Nov 2002, William A. Rowe, Jr. wrote:

> At 05:54 PM 11/20/2002, rbb@apache.org wrote:
>
> >BTW, I want to be 100% clear.  We don't use incomplete types for binary
> >compatibility.  The only reason that we use incomplete types is for
> >portability reasons.  In APR 2.0, I fully expect most of the incomplete
> >types to shrink, and for a good portion of APR to use complete types.
>
> Huh?
>
> Incomplete types have absolutely zero impact on portability, other than
> preventing authors from doing stupid things.  We've never been against
> users making stupid errors, so that can't be it :-)
>
> They are 100% about maintainability.  Cliff's particular change is the
> perfect case-in-point.

Nope.  Sorry, but you are wrong.  You may like them for maintainability
now, but the ENTIRE reason for using them is to stop users from making
stupid mistakes.

> By 2.0, I hope to have helped flesh out a reasonably portable way
> of introducing inline accessors, so the entire performance issue
> flies out the window.  Of course, when one enables the inlines, one
> binds oneself to an exact version of APR (al la HTTP's own internal
> build of APR).  If one does not use inlines, then you have complete
> freedom to use different APR versions as a dynamic library.
>
> Incomplete types were the correct initial design decision, and they
> will continue to be a good decision.

Nope.  Incomplete types were a good first start, but a combination of
complete and incomplete is the best design.

Ryan


Re: apr_mmap_t

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 05:54 PM 11/20/2002, rbb@apache.org wrote:

>BTW, I want to be 100% clear.  We don't use incomplete types for binary
>compatibility.  The only reason that we use incomplete types is for
>portability reasons.  In APR 2.0, I fully expect most of the incomplete
>types to shrink, and for a good portion of APR to use complete types.

Huh?

Incomplete types have absolutely zero impact on portability, other than
preventing authors from doing stupid things.  We've never been against
users making stupid errors, so that can't be it :-)

They are 100% about maintainability.  Cliff's particular change is the
perfect case-in-point.

By 2.0, I hope to have helped flesh out a reasonably portable way
of introducing inline accessors, so the entire performance issue
flies out the window.  Of course, when one enables the inlines, one
binds oneself to an exact version of APR (al la HTTP's own internal
build of APR).  If one does not use inlines, then you have complete
freedom to use different APR versions as a dynamic library.

Incomplete types were the correct initial design decision, and they
will continue to be a good decision.

Bill


Re: apr_mmap_t

Posted by Cliff Woolley <jw...@virginia.edu>.
On Wed, 20 Nov 2002 rbb@apache.org wrote:

> Stop using an incomplete type for MMAP files.  The only sane way to really
> store an MMAP file as far as I can see is as a void * and a length.  BeOS
> requires another variable, but it doesn't do any harm to expose that too.

Well, there's more to it than that now.  :)  Not that much more in Unix,
but more for sure on Win32 et al.

> What are you adding to MMAPs?

I'm working on a patch for the mmap cleanup problems that have been
plaguing us.  I'll post the patch when it's done (very soon).

--Cliff


Re: Opaque types [was apr_mmap_t]

Posted by rb...@apache.org.

On Mon, 25 Nov 2002, William A. Rowe, Jr. wrote:

> At 11:51 PM 11/24/2002, rbb@apache.org wrote:
> >On Sun, 24 Nov 2002, William A. Rowe, Jr. wrote:
> >>
> >> Obviously, this is skeletal.  It provides that all functions are exported,
> >> and some simple functions may be inlined.
> >
> >I tried to snip some of this, but I couldn't.  This is bogus.  Your header
> >with the inline header needs information that is private to the APR
> >internals.  That means that it can never be included from any file that is
> >public.  At the end of the day, what you are describing above is
> >completely impossible.
>
> Obviously it marries the app to the explicit version of APR.  That is the
> downside of inlining otherwise innocuous operations.

No, it does much more than that.  It completely removes the incompleteness
of the type.  Think it through.  The header that has the inline function
(the private header) needs to have full access to the internals of the
type.  The public header CAN'T have access to the internals of the type.
In order for the inlining to work, the private header needs to be included
from an external file (or the public header, which is the same thing).
This means that once you track it all back, the app has access to the
definition of the incomplete type, making it no longer incomplete.

If you can somehow make this work without removing the incompleteness of
APR, then cool.  But, AFAICS, it isn't possible, because what you want to
do can't be done with incomplete types.

> >> I don't believe what you suggest is portable.  Of course my VC is very happy
> >> to parse a struct def with a pointer to an incomplete type anywhere within
> >> the structure, or an undefined array at the *end* of the structure.  But since
> >
> >Will, please, that is just completely bogus.  Are you honestly telling me
> >that you believe that there is a compiler that doesn't know how to put a
> >pointer to an incomplete type inside of a structure?  If so, then Apache
>
> No, I'm stating that having a partially complete structure is not portable.
>
> The question of an embedded pointer to the apr_foo_internal_t isn't an
> issue, except that the user can allocate an apr_foo_t.  Since I just got
> done earlier today expressing that we don't care that users can shoot
> themselves in the foot, I really don't have any argument to that side
> of your proposal.

So in other words, the idea that I have expressed is completely portable.
Cool, then what was your problem with it?????

> >2.0 isn't portable to that platform.  What I am suggesting is 100%
> >identical to what Apache does by putting an apr_file_t inside the
> >request_rec.  Add to that, we are ALREADY doing what I suggest, this model
> >was used in apr_poll_t, and it works just find.
>
> And it implies that apr_foo_t could be allocated by the user, although they
> cannot create the pointer to apr_foo_internal_t.  Let me digest what you are
> proposing so I can comment on it further.

Yep, and without the pointer to the internal structure, APR is essentially
useless.  So, while it would be possible to allocate the public portion of
the structure, it wouldn't get the user anywhere.  Which means, that
binary compat remains, because APR remains the only valid way to allocate
an APR structure.

The only place that binary compat becomes a problem, is that people can
now pass the structure instead of a pointer to the structure.  But if
somebody wants to shoot themselves in the foot, then go ahead.  All they
will be losing is binary compat, which isn't the end of the world,
especially since we provide an easy way for them to regain it.

> You are suggesting two structures.  I spelled out in the prior message
> and a paragraph above why that was a half-way solution.  The real
> answer is a partially complete structure; which is not ANSI C or any
> flavor thereof.

I obviously missed your explanation of why that is a half-way solution.
I'll try to go back and read it again to find the reasoning, but it may
take a day or two.

Ryan



Re: Opaque types [was apr_mmap_t]

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 11:51 PM 11/24/2002, rbb@apache.org wrote:
>On Sun, 24 Nov 2002, William A. Rowe, Jr. wrote:
>>
>> Obviously, this is skeletal.  It provides that all functions are exported,
>> and some simple functions may be inlined.
>
>I tried to snip some of this, but I couldn't.  This is bogus.  Your header
>with the inline header needs information that is private to the APR
>internals.  That means that it can never be included from any file that is
>public.  At the end of the day, what you are describing above is
>completely impossible.

Obviously it marries the app to the explicit version of APR.  That is the
downside of inlining otherwise innocuous operations.

>> I don't believe what you suggest is portable.  Of course my VC is very happy
>> to parse a struct def with a pointer to an incomplete type anywhere within
>> the structure, or an undefined array at the *end* of the structure.  But since
>
>Will, please, that is just completely bogus.  Are you honestly telling me
>that you believe that there is a compiler that doesn't know how to put a
>pointer to an incomplete type inside of a structure?  If so, then Apache

No, I'm stating that having a partially complete structure is not portable.

The question of an embedded pointer to the apr_foo_internal_t isn't an
issue, except that the user can allocate an apr_foo_t.  Since I just got
done earlier today expressing that we don't care that users can shoot
themselves in the foot, I really don't have any argument to that side
of your proposal.

>2.0 isn't portable to that platform.  What I am suggesting is 100%
>identical to what Apache does by putting an apr_file_t inside the
>request_rec.  Add to that, we are ALREADY doing what I suggest, this model
>was used in apr_poll_t, and it works just find.

And it implies that apr_foo_t could be allocated by the user, although they
cannot create the pointer to apr_foo_internal_t.  Let me digest what you are
proposing so I can comment on it further.

>I am not going to respond to the rest of this message, it is nothing but
>flame bait.

Glad we are having a discussion and not a debate, then.

You are suggesting two structures.  I spelled out in the prior message
and a paragraph above why that was a half-way solution.  The real
answer is a partially complete structure; which is not ANSI C or any
flavor thereof.

>We are still waiting for the real labor to begin.  I has been two days,
>and I am wasting time on the computer.

I hear that ... no rest for the expectant mother -or- father :-) 

Bill


Re: Opaque types [was apr_mmap_t]

Posted by rb...@apache.org.
On Sun, 24 Nov 2002, William A. Rowe, Jr. wrote:

> At 09:24 PM 11/24/2002, rbb@apache.org wrote:
>
> >> I'm not arguing that exposing incomplete types wouldn't speed up APR.
> >> In fact, well written accessors that are inline'able on all modern compilers
> >> would be a beautiful thing.  They would work only for 'private' copies of
> >> the apr library, since the app becomes permanently bound to the explicit
> >> version installed at the time you build it.  That would be fine by httpd and
> >
> >There are very few compilers that can inline functions that are in
> >separate libraries.  In fact, I don't currently know of any.
>
> Of course, there are none that inline functions in libraries.  inline functions
> need to exist in headers.  We might end up with a scenario such as...
>
> apr_mmap.h  [public header]
>   #include "apr.h"
>
>   /* Incomplete type */
>   typedef struct apr_mmap_t apr_mmap_t;
>
>   /* Prototype */
>   APR_DECLARE_INLINE(footype) apr_mmap_foo_get(apr_mmap_t *mmap);
>
>   #if APR_HAS_INLINE && APR_USE_INLINE
>   # include "apr_mmap_inline.h"
>   #endif
>
> apr_mmap_inline.h
>   /* This private header is included for inline declarations.
>    * use of the embedded structures binds your application
>    * to this specific build of APR.  These structures are subject
>    * to change without warning. */
>
>   struct apr_mmap_t {
>     footype foo; ...
>   } apr_mmap_t;
>
>   /* Implementation */
>   APR_DECLARE_INLINE(typefoo) apr_mmap_foo_get(apr_mmap_t *mmap) {
>
>   }
>
> mmap_inline.c
>   #undef APR_USE_INLINE
>   #include "apr_mmap.h"
>
>   /* capture inline functions as exported real functions */
>   #include "apr_mmap_inline.h"
>
> mmap.c
>   #define APR_USE_INLINE  /* if it's not simply global */
>   #include "apr_mmap.h"
>
>   APR_DECLARE(apr_status_t) apr_mmap_create(...) {
>     ...
>   }
>
> Obviously, this is skeletal.  It provides that all functions are exported,
> and some simple functions may be inlined.

I tried to snip some of this, but I couldn't.  This is bogus.  Your header
with the inline header needs information that is private to the APR
internals.  That means that it can never be included from any file that is
public.  At the end of the day, what you are describing above is
completely impossible.

> >> other consumers who don't mind building APR themselves, or for apps
> >> that are built via one of the packaging tools that tracks dependencies,
> >> and will rebuild the entire slew of dependent packages when a new
> >> version of APR is installed.
> >>
> >> Of course, this won't work for deployment across different platforms.
> >
> >I can't understand this statement at all.  Deployment across different
> >platforms doesn't make sense in APR.
>
> Sorry, wrong phrase.  Different target machines.  Think binary builds of
> packages, where the package doesn't distribute APR, but presumes it
> will be installed on the machine.  Binary compatibility to the inlines would
> never fly, so that package builds without APR_USE_INLINE, for example.
> Anyone building the package to include the specific release of APR would
> use inline, of course.

As I said above, your whole inlining concept won't work.  You are trying
to get private data to public headers, which can't happen.

> >> By committing to unwrapping all of the opaque types, you seem hell
> >> bent on ensuring we forever break binary compatibility.  Opaque types
> >
> >Again, what in the world are you talking about?  The design that I
> >suggested actually makes binary compatibility one of it's core goals.  I
> >am a VERY strong believer in binary compat for libraries, so this
> >statement makes little to no sense.  If you look at the design, you will
> >see that it splits the structure into two parts:  1)  The logical
> >component and 2)  The OS specific component.  Because there is an
> >incomplete type in the middle of the structure, an allocator is still
> >required to actually create an instance of a type.  That means that binary
> >compat is easy to get, assuming all new fields are added at the end of the
> >structure.  The only time you will get into trouble is if somebody passes
> >the structure itself instead of a pointer to the structure.  Yes, binary
> >compat would be broken between 1.0 and 2.0, but we never said that was a
> >goal.
>
> I don't believe what you suggest is portable.  Of course my VC is very happy
> to parse a struct def with a pointer to an incomplete type anywhere within
> the structure, or an undefined array at the *end* of the structure.  But since

Will, please, that is just completely bogus.  Are you honestly telling me
that you believe that there is a compiler that doesn't know how to put a
pointer to an incomplete type inside of a structure?  If so, then Apache
2.0 isn't portable to that platform.  What I am suggesting is 100%
identical to what Apache does by putting an apr_file_t inside the
request_rec.  Add to that, we are ALREADY doing what I suggest, this model
was used in apr_poll_t, and it works just find.

I am not going to respond to the rest of this message, it is nothing but
flame bait.

> Let me close by offering that we *better* not find out you are responding
> to list messages while waiting in the delivery room/suite :-)  We can pick
> up the dialog after you get back.  God bless the three of you :-)

We are still waiting for the real labor to begin.  I has been two days,
and I am wasting time on the computer.

Ryan


Re: Opaque types [was apr_mmap_t]

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 09:24 PM 11/24/2002, rbb@apache.org wrote:

>> I'm not arguing that exposing incomplete types wouldn't speed up APR.
>> In fact, well written accessors that are inline'able on all modern compilers
>> would be a beautiful thing.  They would work only for 'private' copies of
>> the apr library, since the app becomes permanently bound to the explicit
>> version installed at the time you build it.  That would be fine by httpd and
>
>There are very few compilers that can inline functions that are in
>separate libraries.  In fact, I don't currently know of any.

Of course, there are none that inline functions in libraries.  inline functions
need to exist in headers.  We might end up with a scenario such as...

apr_mmap.h  [public header]
  #include "apr.h"

  /* Incomplete type */
  typedef struct apr_mmap_t apr_mmap_t;

  /* Prototype */
  APR_DECLARE_INLINE(footype) apr_mmap_foo_get(apr_mmap_t *mmap);

  #if APR_HAS_INLINE && APR_USE_INLINE
  # include "apr_mmap_inline.h"
  #endif

apr_mmap_inline.h 
  /* This private header is included for inline declarations.
   * use of the embedded structures binds your application
   * to this specific build of APR.  These structures are subject
   * to change without warning. */

  struct apr_mmap_t {
    footype foo; ...
  } apr_mmap_t;

  /* Implementation */
  APR_DECLARE_INLINE(typefoo) apr_mmap_foo_get(apr_mmap_t *mmap) {
    
  }

mmap_inline.c
  #undef APR_USE_INLINE
  #include "apr_mmap.h"

  /* capture inline functions as exported real functions */
  #include "apr_mmap_inline.h"

mmap.c
  #define APR_USE_INLINE  /* if it's not simply global */
  #include "apr_mmap.h"

  APR_DECLARE(apr_status_t) apr_mmap_create(...) {
    ...
  }

Obviously, this is skeletal.  It provides that all functions are exported,
and some simple functions may be inlined.

>> other consumers who don't mind building APR themselves, or for apps
>> that are built via one of the packaging tools that tracks dependencies,
>> and will rebuild the entire slew of dependent packages when a new
>> version of APR is installed.
>>
>> Of course, this won't work for deployment across different platforms.
>
>I can't understand this statement at all.  Deployment across different
>platforms doesn't make sense in APR.

Sorry, wrong phrase.  Different target machines.  Think binary builds of
packages, where the package doesn't distribute APR, but presumes it
will be installed on the machine.  Binary compatibility to the inlines would
never fly, so that package builds without APR_USE_INLINE, for example.
Anyone building the package to include the specific release of APR would 
use inline, of course.

>> By committing to unwrapping all of the opaque types, you seem hell
>> bent on ensuring we forever break binary compatibility.  Opaque types
>
>Again, what in the world are you talking about?  The design that I
>suggested actually makes binary compatibility one of it's core goals.  I
>am a VERY strong believer in binary compat for libraries, so this
>statement makes little to no sense.  If you look at the design, you will
>see that it splits the structure into two parts:  1)  The logical
>component and 2)  The OS specific component.  Because there is an
>incomplete type in the middle of the structure, an allocator is still
>required to actually create an instance of a type.  That means that binary
>compat is easy to get, assuming all new fields are added at the end of the
>structure.  The only time you will get into trouble is if somebody passes
>the structure itself instead of a pointer to the structure.  Yes, binary
>compat would be broken between 1.0 and 2.0, but we never said that was a
>goal.

I don't believe what you suggest is portable.  Of course my VC is very happy
to parse a struct def with a pointer to an incomplete type anywhere within
the structure, or an undefined array at the *end* of the structure.  But since
we can't partially define a single structure, we will end up with either a pointer
and two allocs, or some cruft to perform a single allocation and stash a pointer
or 'presume' that the opaque component follows the transparent structure.
Either way, you still end up with a legitimate sizeof() unless the structure
allows some incomplete component at the end of the declaration.  Some
compilers don't. 

>> are goodness for versioning, portability, and extensions of the APR
>> library to other languages (e.g. tcl, perl, java etc, especially those
>> with OO schemas).  We (you especially) did a passable job of making
>> APR map very well to OO schemas.  Please consider their benefits
>> of opaque types before we make plans to wipe them out.
>
>Pay very close attention please.  Using a combination of complete and
>incomplete types allows for all of the advantages of both, with very few
>disadvantages.

Yes, but as I just stated in the last paragraph, I don't believe it's available
to all compilers.

>The idea of wrapping APR in an OO language has never been called out as a
>goal, but even if it is made into a goal, splitting the types into a
>combination of complete and incomplete will not hurt that goal at all.

No, it would not if that facility were portable :-)

Let me close by offering that we *better* not find out you are responding
to list messages while waiting in the delivery room/suite :-)  We can pick
up the dialog after you get back.  God bless the three of you :-)  

Bill


Re: Opaque types [was apr_mmap_t]

Posted by rb...@apache.org.

On Sun, 24 Nov 2002, William A. Rowe, Jr. wrote:

> At 10:07 AM 11/24/2002, rbb@apache.org wrote:
>
> >Let me make myself clear.  I haven't expressed an opinion about whether
> >they should or should not be incomplete.  I have stated why incomplete
> >types were originally used in APR, namely for portability.
>
> Some of us interpreted (rightly or wrongly) that they benefit us in a number
> of ways.  There is more than one [right] opinion on this forum.

Understand something:  The original reason for using incomplete types
ISN'T an opinion.  It is a point of fact.  The reasons that we all like
them is an opinion, and I haven't said anything about that.

> >I have also said that having MMAPs be represented by anything other
> >than a void pointer and a size is a bit ludicrous.
>
> Then you clearly don't understand the implementation.
>
> Win32 must retrieve page-aligned file blocks.  That means that the real
> pointer to the mmap allocation is not the boundry the user requested,
> because we backspace to the last boundry when mapping the region.
> Then we give the user a pointer to their desired offset.
>
> Worse yet, the mmap is not the base kernel object.  All mmap's start
> their lives with a handle to a mapping.  With that handle to the mapping,
> we then map any desired pages into memory.
>
> So your offset/length assertion is, simply put, somewhat bogus.  We
> cannot even release the allocation given that original offset/length, on
> Win32 we must release the page-aligned region, then the mapping handle.
> I'll skip your other comments as they aren't germane to a portable mmap
> implementation.

No, it isn't bogus.  The simple fact is that to the application developer,
an MMAP is a void pointer and an offset.  Anything else that is in the
structure is baggage that the OS requires us to carry.  Shrink the MMAP
structure to what it REALLY means to the developer and hide everything
else.

< snipped the rest, I'll respond in a different message, to split MMAP
from future design>

Ryan



Re: Opaque types [was apr_mmap_t]

Posted by rb...@apache.org.
> >I thought of this a few months ago, but I have been sitting on it while we
> >try to get 1.0 out the door.  This is my vision for how APR 2.0 will be
> >implemented (I was going to propose it to the list after 1.0 had been
> >released).  This makes the API cleaner and smaller, because many of the
> >accessor functions can cleanly be deprecated (Think all of the userdata
> >functions).  It also solves a lot of problems with apr_proc_t's on
> >Windows, where we don't have anyplace to store OS specific information.
> >
> >When I said a few days ago that I had hoped that many of the incomplete
> >types would go away in APR 2.0, this is what I meant.  I don't think we
> >should re-write APR 1.0 now, but if you are going to change MMAPs, then
> >please let's do it the right way.
>
> Your "one right way" seems to be at odds with other's interpretations.

Nobody has said that yet.  In fact, your response is the first one since I
posted the idea.  Regardless, the statement stands.  If you are going to
rewrite MMAPS, then please let's do it the right way.  Whether that way is
what I suggested or not is immaterial.

> I'm not arguing that exposing incomplete types wouldn't speed up APR.
> In fact, well written accessors that are inline'able on all modern compilers
> would be a beautiful thing.  They would work only for 'private' copies of
> the apr library, since the app becomes permanently bound to the explicit
> version installed at the time you build it.  That would be fine by httpd and

There are very few compilers that can inline functions that are in
separate libraries.  In fact, I don't currently know of any.

> other consumers who don't mind building APR themselves, or for apps
> that are built via one of the packaging tools that tracks dependencies,
> and will rebuild the entire slew of dependent packages when a new
> version of APR is installed.
>
> Of course, this won't work for deployment across different platforms.

I can't understand this statement at all.  Deployment across different
platforms doesn't make sense in APR.

> By committing to unwrapping all of the opaque types, you seem hell
> bent on ensuring we forever break binary compatibility.  Opaque types

Again, what in the world are you talking about?  The design that I
suggested actually makes binary compatibility one of it's core goals.  I
am a VERY strong believer in binary compat for libraries, so this
statement makes little to no sense.  If you look at the design, you will
see that it splits the structure into two parts:  1)  The logical
component and 2)  The OS specific component.  Because there is an
incomplete type in the middle of the structure, an allocator is still
required to actually create an instance of a type.  That means that binary
compat is easy to get, assuming all new fields are added at the end of the
structure.  The only time you will get into trouble is if somebody passes
the structure itself instead of a pointer to the structure.  Yes, binary
compat would be broken between 1.0 and 2.0, but we never said that was a
goal.

> are goodness for versioning, portability, and extensions of the APR
> library to other languages (e.g. tcl, perl, java etc, especially those
> with OO schemas).  We (you especially) did a passable job of making
> APR map very well to OO schemas.  Please consider their benefits
> of opaque types before we make plans to wipe them out.

Pay very close attention please.  Using a combination of complete and
incomplete types allows for all of the advantages of both, with very few
disadvantages.

The idea of wrapping APR in an OO language has never been called out as a
goal, but even if it is made into a goal, splitting the types into a
combination of complete and incomplete will not hurt that goal at all.

Ryan



Opaque types [was apr_mmap_t]

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 10:07 AM 11/24/2002, rbb@apache.org wrote:

>Let me make myself clear.  I haven't expressed an opinion about whether
>they should or should not be incomplete.  I have stated why incomplete
>types were originally used in APR, namely for portability.  

Some of us interpreted (rightly or wrongly) that they benefit us in a number
of ways.  There is more than one [right] opinion on this forum.

>I have also said that having MMAPs be represented by anything other 
>than a void pointer and a size is a bit ludicrous.

Then you clearly don't understand the implementation.

Win32 must retrieve page-aligned file blocks.  That means that the real
pointer to the mmap allocation is not the boundry the user requested,
because we backspace to the last boundry when mapping the region.
Then we give the user a pointer to their desired offset.

Worse yet, the mmap is not the base kernel object.  All mmap's start
their lives with a handle to a mapping.  With that handle to the mapping,
we then map any desired pages into memory.

So your offset/length assertion is, simply put, somewhat bogus.  We
cannot even release the allocation given that original offset/length, on
Win32 we must release the page-aligned region, then the mapping handle.
I'll skip your other comments as they aren't germane to a portable mmap
implementation.

>I thought of this a few months ago, but I have been sitting on it while we
>try to get 1.0 out the door.  This is my vision for how APR 2.0 will be
>implemented (I was going to propose it to the list after 1.0 had been
>released).  This makes the API cleaner and smaller, because many of the
>accessor functions can cleanly be deprecated (Think all of the userdata
>functions).  It also solves a lot of problems with apr_proc_t's on
>Windows, where we don't have anyplace to store OS specific information.
>
>When I said a few days ago that I had hoped that many of the incomplete
>types would go away in APR 2.0, this is what I meant.  I don't think we
>should re-write APR 1.0 now, but if you are going to change MMAPs, then
>please let's do it the right way.

Your "one right way" seems to be at odds with other's interpretations.

I'm not arguing that exposing incomplete types wouldn't speed up APR.
In fact, well written accessors that are inline'able on all modern compilers
would be a beautiful thing.  They would work only for 'private' copies of
the apr library, since the app becomes permanently bound to the explicit
version installed at the time you build it.  That would be fine by httpd and
other consumers who don't mind building APR themselves, or for apps
that are built via one of the packaging tools that tracks dependencies,
and will rebuild the entire slew of dependent packages when a new 
version of APR is installed.

Of course, this won't work for deployment across different platforms.
By committing to unwrapping all of the opaque types, you seem hell
bent on ensuring we forever break binary compatibility.  Opaque types
are goodness for versioning, portability, and extensions of the APR
library to other languages (e.g. tcl, perl, java etc, especially those
with OO schemas).  We (you especially) did a passable job of making
APR map very well to OO schemas.  Please consider their benefits 
of opaque types before we make plans to wipe them out.

Bill


Re: apr_mmap_t

Posted by David Reid <dr...@jetnet.co.uk>.
> > If you do change how the MMAP type works, I have a suggestion.  MMAP's
are
> > void *'s with size's, and that is the only way to really make sense of
an
> > MMAP.  Make the new structure something like:
> >
> > /* $(platform)_mmap_t are incomplete types implemented per OS */
> > typedef union {
> >     win_mmap_t *win;
> >     unix_mmap_t *unix;
> >     boes_mmap_t *beos;
> > } apr_mmap_internal_t;
> >
> > struct apr_mmap_t {
> >     apr_pool_t *p;
> >     apr_mmap_internal_t *os_specific;
> >     void *mm;
> >     apr_size_t size;
> >     int is_owner;   /* Is this still necessary? */
> > }
> >
> > This gives all of the benefits of incomplete types (portability and
> > maintainability), but it also opens the truly portable portions of the
> > structure to the developer.  The goal with this design is to make the
> > portable parts of APR portable, and the non-portable parts hidden.
> >
> > I thought of this a few months ago, but I have been sitting on it while
we
> > try to get 1.0 out the door.  This is my vision for how APR 2.0 will be
> > implemented (I was going to propose it to the list after 1.0 had been
> > released).  This makes the API cleaner and smaller, because many of the
> > accessor functions can cleanly be deprecated (Think all of the userdata
> > functions).  It also solves a lot of problems with apr_proc_t's on
> > Windows, where we don't have anyplace to store OS specific information.
>
> Another advantage to this model, is that it becomes simple to fold common
> implementations into a single implementation.  We have a lot of duplicate
> code in APR, because of the way the code is structured[1].

I think it looks like a better option than many of the things I've been
seeing bandied around... Also, shouldn't be hard to implement and is nice
and clean (which was definately one of the original goals of APR).

david



Re: apr_mmap_t

Posted by rb...@apache.org.
> If you do change how the MMAP type works, I have a suggestion.  MMAP's are
> void *'s with size's, and that is the only way to really make sense of an
> MMAP.  Make the new structure something like:
>
> /* $(platform)_mmap_t are incomplete types implemented per OS */
> typedef union {
>     win_mmap_t *win;
>     unix_mmap_t *unix;
>     boes_mmap_t *beos;
> } apr_mmap_internal_t;
>
> struct apr_mmap_t {
>     apr_pool_t *p;
>     apr_mmap_internal_t *os_specific;
>     void *mm;
>     apr_size_t size;
>     int is_owner;   /* Is this still necessary? */
> }
>
> This gives all of the benefits of incomplete types (portability and
> maintainability), but it also opens the truly portable portions of the
> structure to the developer.  The goal with this design is to make the
> portable parts of APR portable, and the non-portable parts hidden.
>
> I thought of this a few months ago, but I have been sitting on it while we
> try to get 1.0 out the door.  This is my vision for how APR 2.0 will be
> implemented (I was going to propose it to the list after 1.0 had been
> released).  This makes the API cleaner and smaller, because many of the
> accessor functions can cleanly be deprecated (Think all of the userdata
> functions).  It also solves a lot of problems with apr_proc_t's on
> Windows, where we don't have anyplace to store OS specific information.

Another advantage to this model, is that it becomes simple to fold common
implementations into a single implementation.  We have a lot of duplicate
code in APR, because of the way the code is structured[1].


Ryan

1]  Back when I created the directory structure for APR, a lot of people
told me this would happen.  I didn't listen.  The duplicate code is my
fault, now I am trying to fix it.


Re: apr_mmap_t

Posted by rb...@apache.org.

On Sat, 23 Nov 2002, Justin Erenkrantz wrote:

> --On Saturday, November 23, 2002 7:31 AM -0800 rbb@apache.org wrote:
>
> > Incomplete types are not the solution to binary compatibility.  The
> > correct way to solve binary compat is to have developers who
> > understand what it means and why it is important.
>
> I'm with Jeff and Will here.  In apr_mmap_t, we're exposing lots of
> OS-dependent structures (see all of the #ifdefs).  There is no good
> reason for having these structures exposed.  If a program
> legitimately needs this information, we should have an
> apr_os_mmap_get or something like that.  Don't care much, but
> apr_mmap_t as it stands right now should be an incomplete type as it
> contains non-portable structure members that is intricately tied to
> the implementation.  That's a no-no.  The structure shouldn't be tied
> to the implementation.  -- justin

Let me make myself clear.  I haven't expressed an opinion about whether
they should or should not be incomplete.  I have stated why incomplete
types were originally used in APR, namely for portability.  I have also
said that having MMAPs be represented by anything other than a void
pointer and a size is a bit ludicrous.

This change was made over 2 years ago, and it was made to allow Apache to
do something.  I can't for the life of me remember what the change was for
in Apache, and we seem to have lost revision history for a lot of the
bucket code.  (I can't even get to the apache-2.0 tree from viewcvs right
now).  The best I can tell, the original implementation of apr_bucket_file
wanted access to the internals of the MMAP type.  That implementation
seems to have been changed though.

If you do change how the MMAP type works, I have a suggestion.  MMAP's are
void *'s with size's, and that is the only way to really make sense of an
MMAP.  Make the new structure something like:

/* $(platform)_mmap_t are incomplete types implemented per OS */
typedef union {
    win_mmap_t *win;
    unix_mmap_t *unix;
    boes_mmap_t *beos;
} apr_mmap_internal_t;

struct apr_mmap_t {
    apr_pool_t *p;
    apr_mmap_internal_t *os_specific;
    void *mm;
    apr_size_t size;
    int is_owner;   /* Is this still necessary? */
}

This gives all of the benefits of incomplete types (portability and
maintainability), but it also opens the truly portable portions of the
structure to the developer.  The goal with this design is to make the
portable parts of APR portable, and the non-portable parts hidden.

I thought of this a few months ago, but I have been sitting on it while we
try to get 1.0 out the door.  This is my vision for how APR 2.0 will be
implemented (I was going to propose it to the list after 1.0 had been
released).  This makes the API cleaner and smaller, because many of the
accessor functions can cleanly be deprecated (Think all of the userdata
functions).  It also solves a lot of problems with apr_proc_t's on
Windows, where we don't have anyplace to store OS specific information.

When I said a few days ago that I had hoped that many of the incomplete
types would go away in APR 2.0, this is what I meant.  I don't think we
should re-write APR 1.0 now, but if you are going to change MMAPs, then
please let's do it the right way.

Ryan


Re: apr_mmap_t

Posted by Justin Erenkrantz <je...@apache.org>.
--On Saturday, November 23, 2002 7:31 AM -0800 rbb@apache.org wrote:

> Incomplete types are not the solution to binary compatibility.  The
> correct way to solve binary compat is to have developers who
> understand what it means and why it is important.

I'm with Jeff and Will here.  In apr_mmap_t, we're exposing lots of 
OS-dependent structures (see all of the #ifdefs).  There is no good 
reason for having these structures exposed.  If a program 
legitimately needs this information, we should have an 
apr_os_mmap_get or something like that.  Don't care much, but 
apr_mmap_t as it stands right now should be an incomplete type as it 
contains non-portable structure members that is intricately tied to 
the implementation.  That's a no-no.  The structure shouldn't be tied 
to the implementation.  -- justin

Re: apr_mmap_t

Posted by rb...@apache.org.

On 23 Nov 2002, Jeff Trawick wrote:

> <rb...@apache.org> writes:
>
> > BTW, I want to be 100% clear.  We don't use incomplete types for binary
> > compatibility.  The only reason that we use incomplete types is for
> > portability reasons.  In APR 2.0, I fully expect most of the incomplete
> > types to shrink, and for a good portion of APR to use complete types.
>
> I want to be 100% clear too :)  One reason we use incomplete types is
> for binary compatibility.  Maybe there was a time when we didn't care
> about that, but those days are over.

Incomplete types are not the solution to binary compatibility.  The
correct way to solve binary compat is to have developers who understand
what it means and why it is important.

Ryan


Re: apr_mmap_t

Posted by Jeff Trawick <tr...@attglobal.net>.
<rb...@apache.org> writes:

> BTW, I want to be 100% clear.  We don't use incomplete types for binary
> compatibility.  The only reason that we use incomplete types is for
> portability reasons.  In APR 2.0, I fully expect most of the incomplete
> types to shrink, and for a good portion of APR to use complete types.

I want to be 100% clear too :)  One reason we use incomplete types is
for binary compatibility.  Maybe there was a time when we didn't care
about that, but those days are over.

-- 
Jeff Trawick | trawick@attglobal.net
Born in Roswell... married an alien...

Re: apr_mmap_t

Posted by rb...@apache.org.
BTW, I want to be 100% clear.  We don't use incomplete types for binary
compatibility.  The only reason that we use incomplete types is for
portability reasons.  In APR 2.0, I fully expect most of the incomplete
types to shrink, and for a good portion of APR to use complete types.

Ryan

On Wed, 20 Nov 2002 rbb@apache.org wrote:

>
>
> On Wed, 20 Nov 2002, Cliff Woolley wrote:
>
> > Does anybody have any idea at all why apr_mmap_t is not an incomplete
> > type?  I have to change its size and that's a bit of a PITA because, while
> > it SHOULD not, it COULD break binary compat for stupid modules who use
> > sizeof(apr_mmap_t) for something.
>
>  Revision 1.11 / (view) - annotate - [select for diffs] , Fri Jul 7
> 19:14:52 2000 UTC (2 years, 4 months ago) by rbb
> Branch: MAIN
> Changes since 1.10: +14 -1 lines
> Diff to previous 1.10 (colored)
> Stop using an incomplete type for MMAP files.  The only sane way to really
> store an MMAP file as far as I can see is as a void * and a length.  BeOS
> requires another variable, but it doesn't do any harm to expose that too.
> This cleans up some code for Apache, and it makes sense IMHO.
>
> What are you adding to MMAPs?
>
> Ryan
>
>


Re: apr_mmap_t

Posted by rb...@apache.org.

On Wed, 20 Nov 2002, Cliff Woolley wrote:

> Does anybody have any idea at all why apr_mmap_t is not an incomplete
> type?  I have to change its size and that's a bit of a PITA because, while
> it SHOULD not, it COULD break binary compat for stupid modules who use
> sizeof(apr_mmap_t) for something.

 Revision 1.11 / (view) - annotate - [select for diffs] , Fri Jul 7
19:14:52 2000 UTC (2 years, 4 months ago) by rbb
Branch: MAIN
Changes since 1.10: +14 -1 lines
Diff to previous 1.10 (colored)
Stop using an incomplete type for MMAP files.  The only sane way to really
store an MMAP file as far as I can see is as a void * and a length.  BeOS
requires another variable, but it doesn't do any harm to expose that too.
This cleans up some code for Apache, and it makes sense IMHO.

What are you adding to MMAPs?

Ryan