Re: [Resend] wininet patch

[Alexandre Julliard]

Tim Hentenaar  writes:
> /***********************************************************************
> -
>       FtpGetCurrentDirectoryW (WININET.@)
> -
> -
> - Retrieves the current directory
> -
> - Retrieves the current directory (unicode)
> -
> - Implemented 5/17/2004 Tim Hentenaar

Please don't add changelog information in the code, that's what the
CVS log and the Changelog file are for.

[Resend] wininet patch

[Robert Shearman]

Alexandre Julliard wrote:
> Tim Hentenaar  writes:
> >
>
> /***********************************************************************
> > -
> >       FtpGetCurrentDirectoryW (WININET.@)
> > -
> > -
> > - Retrieves the current directory
> > -
> > - Retrieves the current directory (unicode)
> > -
> > - Implemented 5/17/2004 Tim Hentenaar
>
> Please don't add changelog information in the code, that's what the
> CVS log and the Changelog file are for.

Some other comments too:

+/***********************************************************************
+ *           InternetGetLastResponseInfoW (WININET.@)
+ *
+ * Return last wininet error description on the calling thread
+ * Implemented 5/17/2004 Tim Hentenaar 
+ *
+ * RETURNS
+ *    TRUE on success of writing to buffer
+ *    FALSE on failure
+ *
+ */
+BOOL WINAPI InternetGetLastResponseInfoW(LPDWORD lpdwError,
+    LPWSTR lpszBuffer, LPDWORD lpdwBufferLength)
+{
+    LPWITHREADERROR lpwite = (LPWITHREADERROR)TlsGetValue(g_dwTlsErrIndex);
+    TRACE("\n");
+    *lpdwError = lpwite->dwError;
+    if (lpwite->dwError)
+    {
+    	int i;
+	for (i=0;i<strlen(lpwite->response);i++)
+		lpszBuffer[i] = (WCHAR)lpwite->response[i];

MultiByteToWideChar would be a better choice here.

+	*lpdwBufferLength = lstrlenW(lpszBuffer);

Potential buffer overrun here. lpdwBufferLength is IN and OUT according to
MSDN. So you should use this value to determine the maximum number of
characters to write.

+    } else *lpdwBufferLength = 0;
+    return TRUE;
+}

/***********************************************************************
+ *           InternetFindNextFileW (WININET.@)
+ *
+ * Continues a file search from a previous call to FindFirstFile
+ * Implemented 5/17/2002 Tim Hentenaar 
+ *
+ * RETURNS
+ *    TRUE on success
+ *    FALSE on failure
+ *
+ */
+BOOL WINAPI InternetFindNextFileW(HINTERNET hFind, LPVOID lpvFindData)
+{
+	return InternetFindNextFileA(hFind,lpvFindData);
+}

You can't do this. lpvFindData is usually a pointer to WIN32_FIND_DATAW, and
so you need to convert it properly. The best idea would probably be to copy
the InternetFindNextFileA function here and unicodify. Another common
technique is to create a common function that takes a flag indicating
whether it is unicode and call this from both the A and W functions. Then
you would have portions of code like this:

if (unicode)
    MultiByteToWideChar(...);
else
    strcpy(...);

You also haven't updated the .spec file with the new functions you have
added (e.g. InternetSetStatusCallbackW), so they will not get used.

Rob

[Resend] wininet patch

[Kevin Koltzau]

In an attempt to prevent duplicated effort on this also, I actually have most of
this already implemented in my tree, there is a bit more work left
to do (about a weeks worth at my current pace) to fully complete

On Tuesday 18 May 2004 08:10 pm, Robert Shearman wrote:
> > /***********************************************************************
> > > -
> > >       FtpGetCurrentDirectoryW (WININET.@)
> > > -
> > > -
> > > - Retrieves the current directory
> > > -
> > > - Retrieves the current directory (unicode)
> > > -
> > > - Implemented 5/17/2004 Tim Hentenaar
> >
> > Please don't add changelog information in the code, that's what the
> > CVS log and the Changelog file are for.
>
> Some other comments too:

...
> You can't do this. lpvFindData is usually a pointer to WIN32_FIND_DATAW, and
> so you need to convert it properly. The best idea would probably be to copy
> the InternetFindNextFileA function here and unicodify. Another common
> technique is to create a common function that takes a flag indicating
> whether it is unicode and call this from both the A and W functions. Then
> you would have portions of code like this: