On Thu, Jun 22, 2017 at 7:18 PM, Daniel Gustafsson <dan...@yesql.se> wrote: > Good point. I’ve attached a new version which issues a NOTICE on truncation > and also addresses all other comments so far in this thread. Thanks a lot for > the early patch reviews! > > cheers ./daniel
I have done an initial review of the patch. I have some comments/suggestions. +int +GetCancelMessage(char **buffer, size_t buf_len) +{ + volatile BackendCancelShmemStruct *slot = MyCancelSlot; + int msg_length = 0; + Returned value of this function is never used, better to convert it to just void. ------- +bool +HasCancelMessage(void) +{ + volatile BackendCancelShmemStruct *slot = MyCancelSlot; +/* + * Return the configured cancellation message and its length. If the returned + * length is greater than the size of the passed buffer, truncation has been + * performed. The message is cleared on reading. + */ +int +GetCancelMessage(char **buffer, size_t buf_len) I think it will be good to merge these two function where GetCancelMessage will first check whether it has the message or not if it does then allocate the buffer of size slot->len and copy the slot message to allocated buffer otherwise just return NULL. So it will look like "char *GetCancelMessage()" --------- + SpinLockAcquire(&slot->mutex); + strlcpy(*buffer, (const char *) slot->message, buf_len); strlcpy(*buffer, (const char *) slot->message, slot->len); Isn't it better to copy only upto slot->len, seems like it will always be <= buf_len, and if not then we can do min(buf_len, slot->len) ---- -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers