Re: Replace remaining StrNCpy() by strlcpy()

Peter Eisentraut <peter.eisentraut@2ndquadrant.com>

From: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: David Rowley <dgrowleyml@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2020-08-08T05:57:53Z
Lists: pgsql-hackers

Attachments

On 2020-08-05 17:49, Tom Lane wrote:
> However I do see one remaining nit to pick, in CreateInitDecodingContext:
>   
>   	/* register output plugin name with slot */
>   	SpinLockAcquire(&slot->mutex);
> -	StrNCpy(NameStr(slot->data.plugin), plugin, NAMEDATALEN);
> +	namestrcpy(&slot->data.plugin, plugin);
>   	SpinLockRelease(&slot->mutex);
> 
> This is already a pro-forma violation of our rule about "only
> straight-line code inside a spinlock".  Now I'm not terribly concerned
> about that right now, and the patch as it stands is only changing things
> cosmetically.  But if you modify namestrcpy to do pg_mbcliplen then all
> of a sudden there is a whole lot of code that could get reached within
> the spinlock, and I'm not a bit happy about that prospect.

fixed

> BTW, while we're here I think we ought to change namecpy and namestrcpy
> to return void (no caller checks their results) and drop their checks
> for null-pointer inputs.  AFAICS a null pointer would be a caller bug in
> every case, and if it isn't, why is failing to initialize the
> destination an OK outcome?  I find the provisions for nulls in namestrcmp
> pretty useless too, although it looks like at least some thought has
> been spent there.

fixed

I removed namecpy() altogether because you can just use struct assignment.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Commits

  1. Replace remaining StrNCpy() by strlcpy()