Hi,
On Tue, Aug 26, 2025 at 05:50:34PM -0500, Sami Imseih wrote:
> fixed the issues mentioned above in v13.
>
> > We probably need to do the sprintf/strcpy before LWLockNewTrancheId().
> > Also, I'm thinking we should just use the same tranche for both the DSA and
> > the dshash table [0] to evade the DSMR_DSA_TRANCHE_SUFFIX problem, i.e.,
> > those tranche names potentially require more space.
>
> v13 also includes a separate patch for this. It removes the
> DSMR_DSA_TRANCHE_SUFFIX, and dsh and dsa now use the same
> user defined tranche. The tranche name is also limited to
> the max length of a named tranche, MAX_NAMED_TRANCHES_NAME_LEN,
> coming from lwlock.h
Thanks for the new version.
=== 1
+LWLockNewTrancheId(const char *tranche_name)
{
- int result;
- int *LWLockCounter;
+ int tranche_id,
+ index,
+ *LWLockCounter;
+ Size tranche_name_length = strlen(tranche_name) + 1;
We need to check if tranche_name is NULL and report an error if that's the case.
If not, strlen() would segfault.
=== 2
+ if (tranche_name_length > MAX_NAMED_TRANCHES_NAME_LEN)
+ elog(ERROR, "tranche name too long");
I think that we should mention in the doc that the tranche name is limited to
63 bytes.
=== 3
- /* Convert to array index. */
- tranche_id -= LWTRANCHE_FIRST_USER_DEFINED;
+ tranche_id = (*LWLockCounter)++;
+ index = tranche_id - LWTRANCHE_FIRST_USER_DEFINED;
- /* If necessary, create or enlarge array. */
- if (tranche_id >= LWLockTrancheNamesAllocated)
+ if (index >= MAX_NAMED_TRANCHES)
{
- int newalloc;
+ SpinLockRelease(ShmemLock);
+ elog(ERROR, "too many LWLock tranches registered");
+ }
- newalloc = pg_nextpower2_32(Max(8, tranche_id + 1));
+ /* Directly copy into the NamedLWLockTrancheArray */
+ strcpy((char *) NamedLWLockTrancheArray + (index *
MAX_NAMED_TRANCHES_NAME_LEN),
+ tranche_name);
I was skeptical about using strcpy() while we hold a spinlock. I do see some
examples with strlcpy() though (walreceiver.c for example), so that looks
OK-ish.
Using strcpy() might be OK too, as we already have validated the length, but
maybe
it would be safer to switch to strlcpy(), instead?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
