-----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.
-
Stefan Dösinger