-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
I am OK with this patch in principle if you believe that it is an improvement. It reduces the code size, but on the other hand it means that you're having tiny allocations for DWORDs and GUIDs. Keep in mind that such allocations mean overhead in memory use, run time and memory fragmentation.
I'd like to point out an alternative approach: You can give struct component a variable size, similar to how struct wined3d_private_data in include/wine/wined3d.h works.
But overall I think this is a minor detail considering the overall state of the directplay libs and that there are more important issues to solve. I don't want to stand in your way by arguing about small things like this. I can't promise Alexandre agrees though.
Either way, two style comments: The test is a good test to add, but should be in its own patch.
Am 2015-06-10 um 06:41 schrieb Alistair Leslie-Hughes:
found = TRUE;
if(entry->type == DPNA_DATATYPE_STRING_ANSI)heap_free(entry->data.ansi);else if(entry->type == DPNA_DATATYPE_STRING)heap_free(entry->data.string);else if(entry->type == DPNA_DATATYPE_BINARY)heap_free(entry->data.binary);
heap_free(entry->data); break;
The added heap_free line is indented by 11 chars.
Cheers, Stefan
Hi Stefan,
Thanks for the review.
The patch started out at fixing an issue I found in Duplicate, however it turned into simplifying the code, thus the test you commented on.
After looking at wined3d_private_data, I see the the way it should be handled and will provide a cleaner patch to fix the issue I found initially.
Best Regards Alistair Leslie-Hughes
On 10/06/15 19:17, Stefan Dösinger wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
I am OK with this patch in principle if you believe that it is an improvement. It reduces the code size, but on the other hand it means that you're having tiny allocations for DWORDs and GUIDs. Keep in mind that such allocations mean overhead in memory use, run time and memory fragmentation.
I'd like to point out an alternative approach: You can give struct component a variable size, similar to how struct wined3d_private_data in include/wine/wined3d.h works.
But overall I think this is a minor detail considering the overall state of the directplay libs and that there are more important issues to solve. I don't want to stand in your way by arguing about small things like this. I can't promise Alexandre agrees though.
Either way, two style comments: The test is a good test to add, but should be in its own patch.
Am 2015-06-10 um 06:41 schrieb Alistair Leslie-Hughes:
found = TRUE;
if(entry->type == DPNA_DATATYPE_STRING_ANSI)heap_free(entry->data.ansi);else if(entry->type == DPNA_DATATYPE_STRING)heap_free(entry->data.string);else if(entry->type == DPNA_DATATYPE_BINARY)heap_free(entry->data.binary);
heap_free(entry->data); break;The added heap_free line is indented by 11 chars.
Cheers, Stefan
-----BEGIN PGP SIGNATURE----- Version: GnuPG v2
iQIcBAEBAgAGBQJVeAC0AAoJEN0/YqbEcdMws9oP/RyusEIcZnV4GWTAmfaSKfFn 0iheACWwmeL3GlWeqmY51S8ImuKcoD0HoZ/vQm6jMhTTJFK3jENVJkMk/Mjx8Kk6 ZPcit6nPB184lDgzLXV0gHbMlPkFxdTPJi++1ZfKa9LskJ3zIrgF7LcJFidtCSeG 7yHdg6C0BiQH4oXjBzY+fVRzaQluKEihncaa2tYdErfMUCzUf+RAi99/tnete2Re EiejXowzg5HcxjBHMupsGEcFocUnXtUlPzJf0UIawbptxzPYXk3H0VYPohIrLVm6 f/chcdKlvAi5xdUlCuF4nLw9TQxyC5h3mf5p7l/JdDbgRE8knCO1w/HrzOHJJJRB TnkU5EdrnIc3Bx6Tw/kQGTKsM3SAuSapdjBsQRracNaQVzdqYKfsMPaiQeRVmdQO LJbyeECFw3l3opf/LFd/TUguDws2hChASaBAOf25vorhD6wH8fnaoNnEh1qMtBH5 x1jJjxB+B0Uwa4nYKProoOc0Qw0/zoaVlv3vCKiiEggz+BkXEuYb58ZDysSg4A1w JVO0S6GAfumFPWvHMtoIEPsFxiPl6xHiwXsKzrHc8K5EpUTZ2Jcv0lam53x39Tb1 cJmluv1SyxrdrTWT1nCtgg+ma8ezid8HEpOj+aIWVwYcKwR9uF09jvDEaAzwSuAr V6ogftbNHriUDl15YrO+ =zne3 -----END PGP SIGNATURE-----
-
Alistair Leslie-Hughes -
Stefan Dösinger