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