dpnet: Implement IDirectPlay8Address GetComponentByIndex (try 4)
Stefan Dösinger
stefandoesinger at gmail.com
Tue Jan 6 08:53:02 CST 2015
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi,
You have style inconsistencies in the patch. I won't point them out individually and just focus on suggestions on the code itself.
Overall: Is there a way to remove components? I don't see a method that supports that, and AddComponent with a NULL pointer returns an error, so I guess components can't be removed.
Am 2015-01-06 um 08:59 schrieb Alistair Leslie-Hughes:> +static void add_component(IDirectPlay8AddressImpl *This, struct component *item)
> +{
> + if(!This->comp_count)
> + This->components = HeapAlloc( GetProcessHeap(), 0, sizeof(struct component *) * This->comp_count+1 );
> + else
> + This->components = HeapReAlloc( GetProcessHeap(), 0, This->components, sizeof(struct component *) * This->comp_count+1 );
How many components are usually added? If it is just a small number (say, < 10) this is OK. Otherwise it would be more efficient to grow the array by a factor (e.g. duplicate the array size each time you grow it), so you make growing an O(log(n)) instead of O(n).
> + int i;
> + struct component *entry;
>
> - LIST_FOR_EACH_ENTRY_SAFE(entry, entry2, &This->components, struct component, entry)
> + for(i=0; i < This->comp_count; i++)
Mismatching data type. This->comp_count is a DWORD, i is an int. Please use DWORD consistently. (Using DWORD for the component is mandated by the parameters to GetComponentByIndex and return value of GetNumComponents)
> + 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;
> + case DPNA_DATATYPE_STRING:
> + memcpy(pvBuffer, &entry->data.string, entry->size);
> + break;
> + case DPNA_DATATYPE_STRING_ANSI:
> + memcpy(pvBuffer, &entry->data.ansi, entry->size);
> + break;
> + case DPNA_DATATYPE_BINARY:
> + memcpy(pvBuffer, &entry->data.binary, entry->size);
> + break;
> + }
You can use assignments for DPNA_DATATYPE_DWORD and DPNA_DATATYPE_GUID. The same applies to the existing code in GetComponentByName. AddComponent already uses assignments.
> --- a/dlls/dpnet/dpnet_private.h
> +++ b/dlls/dpnet/dpnet_private.h
> @@ -94,7 +94,8 @@ struct IDirectPlay8AddressImpl
> GUID SP_guid;
> BOOL init;
>
> - struct list components;
> + struct component **components;
> + DWORD comp_count;
Do you need the extra indirection? If the number of components is small you can work without it I think.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2
iQIcBAEBAgAGBQJUq/bOAAoJEN0/YqbEcdMwOnMP/RnnAWVfyTuK2ABnRqDLW9/1
DydTHhjeOpm1FDKJTy7ebzqU76HDL7YWJOqwSNYPhtr8e3omXt5nApptwVjOBst0
lpRxDju4sPGo9GYePy5w39TI/w78gPLEGLVHpIiV5GlMz+YRpgxKI05Q90jkGtfo
whPpQ9ns1xiXz+d07iJnPn8GnEjWeTkIN2NnY6Nj+ARQ8rmUNSn3YR4ZGtZfuzrt
nKUurOt04FUAqQzDNYNbdtpybYQq7SSVQr8vWpJAJOpedq7oNyCEdjC8Zl5Yitl5
bg6a4oTvl7PgGmYOSvNVzsh6L0dYv0HpjLZsfLUo97ULXRsCFgfX8yNogrCSq8o0
2KoZSWEhehn1RV+WCywMbxHxly5ZvkyEr9g9/h+8+7cpPrdg8TeoaMUjtjUxrqra
fLDfLtGnMGs4DGiFmleRR3lBFmr8UMVkP4/YLEJ1HPKNdAAwXfQso6/ODmdvC78W
5sRfY/pSlLEn93szGfx+hunpUX30c2XkGmuBElLJQsQgVeoijmsNhUNOLUdhTx9k
wPhslaQ+HCtR76KVghvQzN3iBHPuO+O0dfH4PWz5/7Grq5/pIhfbWcMCVJJM3d9u
CNjKAOkw+NzPPsdLyJZsmMWWvv+BJ039l15xjMTmWxG2uH/QgaiBtvDyD2/u3KxJ
MgsfOYAWEYondC/Jllaz
=uIjr
-----END PGP SIGNATURE-----
More information about the wine-devel
mailing list