-----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-----