apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "William A. Rowe, Jr." <wr...@rowe-clan.net>
Subject RE: cvs commit: apr/mmap/win32 mmap.c
Date Thu, 07 Dec 2000 14:09:36 GMT
> 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.)

View raw message