Hi,
I've begun reviewing this patch and have a few questions listed below:
1. + if (i < LWLockTrancheNames.shmem->allocated && DsaPointerIsValid(old_ptrs[i]))
Should an assert be used for the second condition instead?
Since for i < LWLockTrancheNames.shmem->allocated, the dsa pointer is expected to be valid.
2. copied_ptr = dsa_allocate(LWLockTrancheNames.dsa, len);
+
+ copied_addr = dsa_get_address(LWLockTrancheNames.dsa, copied_ptr);
+ memcpy(copied_addr, old_name, len);
+
+ new_ptrs[i] = copied_ptr;
+
+ /* free old tranche names */
+ dsa_free(LWLockTrancheNames.dsa, old_ptrs[i]);
Why is it necessary to allocate a new dsa_pointer for tranche names that are the same size and then
free the old one?
Is there a reason we can't just assign new_ptrs[i] = old_ptrs[i]?
3.
>Additionally, while users should not pass arbitrary tranche IDs (that is,
>IDs not created via LWLockNewTrancheId) to LWLockInitialize, nothing
>technically prevents them from doing so. Therefore, we must continue to
>handle such cases gracefully by returning a default "extension" tranche name.
Would it be possible to update LWLockInitialize so that it checks if tranche_id is
already registered in the dsa, and if not, registers it during the LWLockInitialize() process?
Thank you,
Rahila Syed
