This is the mail archive of the cygwin@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: mmap() and gcc precompiled headers


On Thu, Jul 03, 2003 at 06:23:26PM -0700, Earl Chew wrote:
> o The if (addr) test is redundant (it will always succeed -- I think
>   you meant to write *addr, but I think it's better to simply do
>   away with the test since it doesn't buy anything).

I've just fixed the conditional for now.  I think I know what you have
in mind but, well, the job of the conditional is clear, especially with
the comment above.  Sure it could also be expressed in another way. 
See below.

> o I think there is a problem with the address arithmetic in the match()
>   method used by munmap(). Compare the code in list::match (__off64_t
>   off, DWORD len) with list::match (caddr_t addr, DWORD len,
>   __off32_t start).
> 
>   I believe the address relationships are:
> 
>         get_offset()                get_size()
>       <-------------> <----------------------------------->
>      +----------------------------------------------------+
>      |               :                                    |
>      +----------------------------------------------------+
>      ^                 ^ <----------------------------->
>      |                 |              len
>   get_address()        |
>                      addr

Uhm, no.

get_size() is the value matching gran_len in mmap64, get_address() matches
base, returned by fh->mmap().  So get_address() + get_size() returns the
full amount of memory returned by MapViewOfFileEx().  addr and len are
represented by the map_map (the bit field, flagging used and free areas in
the map).  That means that addr+len must actually be contained in
get_address()+get_size().

I'm sorry but I can't use your patch unless you've signed a copyright
assignment.  I saw that it's too long to go in w/o this, unfortunately,
so I didn't have a close look not to get tainted.  Please see
http://cygwin.com/contrib.html.  A ChangeLog entry would be required
as well.  

Thanks,
Corinna

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]