You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2004/09/22 22:29:31 UTC

Re: cvs commit: apr/network_io/win32 sendrecv.c

At 01:21 PM 9/22/2004, ake@apache.org wrote:
>  --- ap_regkey.c       9 Feb 2004 20:40:49 -0000       1.11
>  +++ ap_regkey.c       22 Sep 2004 18:21:29 -0000      1.12
>  @@ -185,7 +185,7 @@
>        */
>       LONG rc;
>       DWORD type;
>  -    DWORD size = 0;
>  +    apr_size_t size = 0;
>       
>   #if APR_HAS_UNICODE_FS
>       IF_WIN_OS_IS_UNICODE 
>  @@ -201,7 +201,7 @@
>           else if (valuelen)
>               return APR_ENAMETOOLONG;
>           /* Read to NULL buffer to determine value size */
>  -        rc = RegQueryValueExW(key->hkey, wvalname, 0, &type, NULL, &size);
>  +        rc = RegQueryValueExW(key->hkey, wvalname, 0, &type, NULL, (DWORD *)&size);

Allen!!!  This is insidiously evil!!!  This change makes assumptions
of the endianness will bite us some day!

Please don't use casts :(  Certainly, never recast pointer types
in this manner.  Can you explain your choice of type below, I'm
not grokking it... (perhaps we mean ULONG_PTR instead?)

>  --- readwrite.c       13 Feb 2004 09:38:27 -0000      1.80
>  +++ readwrite.c       22 Sep 2004 18:21:30 -0000      1.81
>  @@ -68,7 +69,8 @@
>           file->pOverlapped->OffsetHigh = (DWORD)(file->filePtr >> 32);
>       }
>   
>  -    rv = ReadFile(file->filehand, buf, len, nbytes, file->pOverlapped);
>  +    *nbytes = 0;
>  +    rv = ReadFile(file->filehand, buf, len, (LPDWORD)nbytes, file->pOverlapped);
>   
>       if (!rv) {
>           rv = apr_get_os_error();
>  @@ -85,7 +87,7 @@
>               switch (rv) {
>               case WAIT_OBJECT_0:
>                   GetOverlappedResult(file->filehand, file->pOverlapped, 
>  -                                    nbytes, TRUE);
>  +                                    (LPDWORD)nbytes, TRUE);
>                   rv = APR_SUCCESS;
>                   break;
>               case WAIT_TIMEOUT:
>  @@ -309,7 +311,7 @@
>                   rv = WaitForSingleObject(thefile->pOverlapped->hEvent, INFINITE);
>                   switch (rv) {
>                       case WAIT_OBJECT_0:
>  -                        GetOverlappedResult(thefile->filehand, thefile->pOverlapped, nbytes, TRUE);
>  +                        GetOverlappedResult(thefile->filehand, thefile->pOverlapped, (LPDWORD)nbytes, TRUE);
>                           rv = APR_SUCCESS;
>                           break;
>                       case WAIT_TIMEOUT:

Otherwise nice work :)

I wish you would have broken out the xmitfile bit, it was a pretty
comprehensive change in and of itself.

Bill