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
