Re: Regarding my old mmap patch
"Nuno Lopes" <[email protected]> Mon, 4 Sep 2006 11:16:17 +0100
| Newsgroups | gmane.comp.web.html-tidy.devel |
|---|---|
| Message-ID | <004a01c6d00b$2944f300$0100a8c0@pc07653> |
Just a dew thoughts about your patch:
I would replace this:
+ data->view = MapViewOfFile( data->map, FILE_MAP_READ,
+ (DWORD)( data->pos >> 32 ),
+ (DWORD)data->pos, numb );
with a simple:
+ data->view = MapViewOfFile( data->map, FILE_MAP_READ, 0, 0, 0);
because you already know that pos is zero and that way you don't have to
play with the system granularity (thus removing some code you have there and
also reduces the structure size :P).
also I don't think you need the pos struct member (it seems pretty redudant
to me).
This block also introduces potential mem leaks if GetFileSizeEx or
CreateFileMapping fail but not the malloc:
+ if ( !fin || !GetFileSizeEx( fp, (LARGE_INTEGER*)&fin->size ) ||
+ fin->size <= 0 )
+ {
+ return -1;
+ }
+
+ fin->map = CreateFileMapping( fp, NULL, PAGE_READONLY, 0, 0, NULL );
+
+ if ( !fin->map )
+ return -1;
This is just a little review. I haven't tested the patch myself, but it
looks good at first glance.
Regards,
Nuno
----- Original Message -----
>I finally got some more time and have submitted a new patch, this time
> with the other more controversial stuff taken out, leaving only the
> win32 memory mapping.
>
> The patch works off of tonight's CVS.
>
> http://sourceforge.net/tracker/index.php?func=detail&aid=1551769&group_id=27659&atid=390965
>
> --
> Cory Nelson
> http://www.int64.org
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642