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