mswsock.dll: TranmitFile - new way [1/2]

[Juan Lang]

Hi Lukazs, I'm afraid this still isn't ready for inclusion:

diff -u -r1.3 Makefile.in
--- dlls/mswsock/Makefile.in	9 May 2005 14:42:33 -0000	1.3
+++ dlls/mswsock/Makefile.in	27 Jun 2006 23:13:09 -0000
@@ -4,13 +4,16 @@
 VPATH     = @srcdir@
 MODULE    = mswsock.dll
 IMPORTLIB = libmswsock.$(IMPLIBEXT)
-IMPORTS   = ws2_32 iphlpapi kernel32
+IMPORTS   = ws2_32 kernel32 
+
This change looks fine, but it could, and therefore should, be sent as a separate patch.

/***********************************************************************
- *		TransmitFile (MSWSOCK.@)
+ *              WSAAddressToStringW                      (MSWSOCK.@)
  *
- * This function is used to transmit a file over socket.
+ * Transmit file via socket. Header  / Footer data is also supported.
  *
- * TODO
- *  This function is currently implemented as a stub.
+ * PARAMS
+ *  hSocket [I]    Socket for sending the data. Not handle.
+ *  hFile      [I]    file to transmit.
+ *  nNumberOfBytesToWrite     [I]    How much data to write. Optional 
+ *  nNumberOfBytesPerSend  [I] Maximum length of single packet (usefull for communication protocols)
+ *  lpOverlapped [I/O] used for overlapped operations 
+ *  lpTransmitBuffers [I] data to send as header/tail usefull for communication protocols)

Same here.  This looks good, but perhaps send this as a documentation patch.
Lining up the text better would be nice too.

+    if(!nNumberOfBytesToWrite)
+        nNumberOfBytesToWrite = -1; /* should be OK */

Why wouldn't it be?  The comment doesn't tell me anything, so make it say what it means, or omit it.

+            if(!dwRetVal)
+            {
+                WSASetLastError(WSAEACCES);
+                goto e_exit;

Are you sure WSAEACCES should be set on any ReadFile error?  I suspect ReadFile already sets
the last error, so a goto e_exit should be sufficient.

+            }
+    if(dwFlags & TF_DISCONNECT) 
+    
+ok_exit:
+    if(lpOverlapped)
+        lpOverlapped->Internal = dwTotalBytesSent; /* FIXME */

The first if is missing a then block.
Fix what?  The comment needs to be clear about what needs to be fixed.

In the tests patch,
+/** #define RUN_BIG_FILE_TEST  /**/

Eck.  That's ugly.  At least my editor complains about the nested comment.
Just omit dead code from your patch.


+#ifndef SAFE_RELEASE

Why do you need the ifndef protection?  It's not going to be defined anywhere else, so just define it.

+#define SAFE_RELEASE(x) {if(x) {free(x); x=NULL;}}

If you must use multi-statement macros, please use something like:
do { ... } while 0
instead.  Note the lack of closing semicolon.  See e.g. http://c-faq.com/cpp/multistmt.html

+#define SAFE_HRELEASE(x) {if(x!=INVALID_HANDLE_VALUE) {CloseHandle(x); x=INVALID_HANDLE_VALUE;}}

What does that help?  CloseHandle already handles invalid handles.  Please just use that instead.

+    HANDLE    hFOutput     = INVALID_HANDLE_VALUE;
+    int     i             = 0;
+    int     size        = 0;
+    DWORD   written      = 0;
+    DWORD   filesize      = 0;
+    BOOL    nBoolRetval = FALSE;

What's the reason for aligning the variable declarations and initializations just so?
It looks like you just used your editor to replace tabs with spaces, without paying attention
to indentation.  We accept all sorts of styles in wine code--but please pick one style.

--Juan