| From: | Yurii Rashkovskii <yrashk(at)gmail(dot)com> | 
|---|---|
| To: | Aleksander Alekseev <aleksander(at)timescale(dot)com> | 
| Cc: | pgsql-hackers(at)lists(dot)postgresql(dot)org | 
| Subject: | Re: [PATCH] Extend the length of BackgroundWorker.bgw_library_name | 
| Date: | 2023-04-24 11:40:10 | 
| Message-ID: | CA+RLCQyvbdkO-pkL5oaxn6ftGYM7OY+U_KvNwaSCRR1ce2yo5A@mail.gmail.com | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
Aleksander,
On Mon, Apr 24, 2023 at 1:01 PM Aleksander Alekseev <
aleksander(at)timescale(dot)com> wrote:
> > The patch is backwards-compatible and ensures that bgw_library_name
> stays *at least* as long as BGW_MAXLEN. Existing external code that uses
> BGW_MAXLEN is a length boundary (for example, in `strncpy`) will continue
> to work as expected.
>
> There is a mistake in the comment though:
>
> ```
> +/*
> + * Ensure bgw_function_name's size is backwards-compatible and sensible
> + */
> +StaticAssertDecl(MAXPGPATH >= BGW_MAXLEN, "MAXPGPATH must be at least
> equal to BGW_MAXLEN");
> ```
>
> library_name, not function_name. Also I think the comment should be
> more detailed, something like "prior to PG17 we used ... but since
> PG17 ... which may cause problems if ...".
>
This is a very good catch and a suggestion. I've slightly reworked the
patch, and I also made this static assertion to have less indirection and,
therefore, make it easier to understand the premise.
Version 2 is attached.
-- 
Y.
| Attachment | Content-Type | Size | 
|---|---|---|
| v2-0001-Extend-the-length-of-BackgroundWorker.bgw_library_na.patch | application/octet-stream | 4.3 KB | 
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Hayato Kuroda (Fujitsu) | 2023-04-24 12:03:05 | RE: [PoC] pg_upgrade: allow to upgrade publisher node | 
| Previous Message | Ajit Awekar | 2023-04-24 11:28:08 | Re: Memory leak in CachememoryContext |