mscoree: Reimplemented GetCORVersion using a helper that checks properly the return buffer; shortened a few FIXMEs (1/4)

[Paul Chitescu]

On Mon, 13 Nov 2006, James Hawkins wrote:
> On 11/13/06, Paul Chitescu <paulc at voip.null.ro> 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