On Mon, 13 Nov 2006, James Hawkins wrote:
On 11/13/06, Paul Chitescu <[EMAIL PROTECTED]> wrote:
Changelog: mscoree: Reimplemented GetCORVersion using a helper that checks
properly the return buffer; shortened a few FIXMEs (1/4)
Can you please put the [x/y] at the beginning of the subject, after
the module name. My mailer cuts off long subjects, so I can't tell
which order I need to look at the patches.
That's a good point - I'll do so in the future.
+#include "config.h"
+#include "wine/port.h"
+#include "wine/library.h"
+#include "wine/debug.h"
Why did you add these includes (besides debug.h)?
Some later added functions require them. This patch is really a larger
patch broken into parts at Alexandre's request.
+/* this helper is needed very often */
This is a very useless comment. Please leave it out. Also, you say
this helper function is needed often, but as it stands, it's only
being used once, and it hides the real purpose of the GetCORVersion
function. Copying to an out parameter is pretty standard for a lot of
the Win32 API, and we don't use helper functions anywhere else in the
Wine code base for this purpose because of reasons stated above, and
because each function usually does something different with the
parameters.
If you look at the API this particular version of copying to a buffer or
failing with E_POINTER repeats a lot. It's just that this is the first
patch in a series.
+static HRESULT copyToWBuffer(LPCWSTR str, LPWSTR pBuf, DWORD cchBuf,
LPDWORD pBufLen)
+{
+ DWORD len;
+ if (!(pBuf && cchBuf))
+ return E_POINTER;
+ len = lstrlenW(str);
+ if (pBufLen)
+ *pBufLen = len;
+ if (cchBuf < len)
+ return ERROR_INSUFFICIENT_BUFFER;
+ lstrcpyW(pBuf, str);
+ return S_OK;
+}
You've totally changed the whitespace style of the file in this helper
function. Please leave GetCORVersion the way it was.
Sorry - my editor replaced some spaces with a TAB. No big deal, only 2
lines to change.
Paul Chitescu
