Re: dpnet: Implement IDirectPlay8Address GetComponentByIndex (try 8)

[Stefan Dösinger]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

There are a few issues with this. They seem vaguely familiar to me, I
think I have raised them before but they got lost in the list to array
conversion:

On 04/15/15 12:07, Alistair Leslie-Hughes wrote:
> @@ -405,9 +405,48 @@ static HRESULT WINAPI IDirectPlay8AddressImpl_GetComponentByIndex(IDirectPlay8Ad
>          const DWORD dwComponentID, WCHAR *pwszName, DWORD *pdwNameLen, void *pvBuffer,
>          DWORD *pdwBufferSize, DWORD *pdwDataType)
>  {
> I assume you're supposed to write the name to pwszName if it is
> non-NULL, and check *pdwNameLen against the string length first.
> +    entry = This->components[dwComponentID];
> +
> +    if(*pdwBufferSize < entry->size)
> +    {
> +        *pdwBufferSize = entry->size;
> +        return DPNERR_BUFFERTOOSMALL;
> +    }
> It seems to me that it should assign *pdwDataType and *pdwNameLen in
> this case. The test should be extended to explicitly check the
> assigned lengths. Your code never sets *pdwNameLen, and it sets
> *pdwDataType only if the buffer is big enough.

The concern about assigning *pdwDataType in the buffer to small case
also applies to GetComponentByName.
> +    switch (entry->type)
> +    {
> +        case DPNA_DATATYPE_DWORD:
> +            memcpy(pvBuffer, &entry->data.value, sizeof(DWORD));
> +            break;
> +        case DPNA_DATATYPE_GUID:
> +            memcpy(pvBuffer, &entry->data.guid, sizeof(GUID));
> +            break;
> Assignments should be enough in these cases. The same applies to the
> existing GetComponentByName code.

A semi-related problem in the existing GetComponentByName code:
> if(!pwszName || !pdwBufferSize || !pdwDataType || (!pvBuffer && pdwBufferSize))
>         return E_POINTER;

The second check for pdwBufferSize is redundant, you already
established it is not NULL. I assume that's supposed to be *pdwBufferSize.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJVLj+3AAoJEN0/YqbEcdMws+UP/0q9wkiDyPPLi5+tbp8NXPKZ
NItf38ksjFeForuNvmRt/Q0MAvQeJ0CSpqoyk3nH9G0EvcNQPXEpBHBCiL7aqUVU
HN0OWc07OANPPrvAVqNRAVVaaAr1v9NFd+CJFNqNf7d0uOj0D1GMJtvJw+1ia06P
qFfAUIIRSf1ay4RocO2HyAsaYwtps15Pib067jBJT/iVriMJQuflB9WRBy6BmKrR
DM0PzySi3+quFjl+tYKUitUeR4RgGsMXUSx8V3l03929Y1duRO0uSfzAs18Mquqh
wovE36jJfYhpBhJa7oAI9CVkNvr/EBMSVzHtBOGWM6mewMI3CGrfn+ZLxjOe1kk3
pIFXfIlbBBTZJYusL8fOxL0itacE2+YB5Opzi2K6TyNfnqWHscR5g7q+JH6iIe4k
/JepaI2mqPsD2RgvM7SKBH2s6qz+cfd7uHWiW+QzzVEqIixbb3eJm1raR9p2UMAb
+aJ9XPYpIeRzy8+BxCXLWhd5GQIFOSlUisQwxbwM9a/d9URFgcLS2KDX3ZjPPBcL
hTOCS3jDiIfjf5lTpl1wj/rb+9zmK0JkpFyf6/EbxZEThP/fNuwSHtIYaY/4UO97
H9bpWYI5x1N7VnXaAFhCy9uwMeJ05ZYj0AhVl70S4Q1pHsFSX5XWumTOpfxXtJhH
WRMpPPalUDabz3fPcAgG
=rxp6
-----END PGP SIGNATURE-----