Обсуждение: Use pg_malloc macros in src/fe_utils
Hi Inspired by [1], I took a stab at converting src/fe_utils to the new pg_malloc macros. Some mostly useless metrics: 31/34 converted, 7 casts removed. meson test looks good for me. 1. https://www.postgresql.org/message-id/flat/CAHut%2BPvpGPDLhkHAoxw_g3jdrYxA1m16a8uagbgH3TGWSKtXNQ%40mail.gmail.com best regards, Henrik
Вложения
On 2/18/26 4:05 PM, Henrik TJ wrote: > Inspired by [1], I took a stab at converting src/fe_utils to the new > pg_malloc macros. Thanks for the patch! Looks like a nice change but why not just fix all instances of it in one swoop? It cannot be that many as there are 166 calls to pg_malloc() and 62 calls to pg_malloc0() after your patch that need to be looked at. -- Andreas Karlsson Percona
On Mon, Feb 23, 2026 at 03:17:52AM +0100, Andreas Karlsson wrote: > Looks like a nice change but why not just fix all instances of it in one > swoop? It cannot be that many as there are 166 calls to pg_malloc() and 62 > calls to pg_malloc0() after your patch that need to be looked at. FWIW, I don't really mind if these changes are proposed gradually, and this looked fine enough on its own. So applied. -- Michael
Вложения
On 2/24/26 4:36 AM, Michael Paquier wrote: > On Mon, Feb 23, 2026 at 03:17:52AM +0100, Andreas Karlsson wrote: >> Looks like a nice change but why not just fix all instances of it in one >> swoop? It cannot be that many as there are 166 calls to pg_malloc() and 62 >> calls to pg_malloc0() after your patch that need to be looked at. > > FWIW, I don't really mind if these changes are proposed gradually, and > this looked fine enough on its own. So applied. Fair, here is a patch which should handle all uses in the frontend code so we follow this pattern consistently to encourage new code to use these macros. When doing this I found two things which I am ot sure what the cleanest way to handle would be so I broke them out into separate patches. 1. What should we do about when we allocate a an array of characters? Would it make sense to use pg_array_alloc() or would that jsut be silly? For example: -pad = (char *) pg_malloc(l + 1); +pad = pg_malloc_array(char, l + 1); 2. I found a small and harmless thinko. The buffer in verify_tar_file() is actually a char * but for some reason the code did the following: buffer = pg_malloc(READ_CHUNK_SIZE * sizeof(uint8)); What should we do about it? Just skip the "sizof(uint8)"? Andreas
Вложения
On Fri, Feb 27, 2026 at 02:15:46AM +0100, Andreas Karlsson wrote: > 1. What should we do about when we allocate a an array of characters? Would > it make sense to use pg_array_alloc() or would that jsut be silly? For > example: > > -pad = (char *) pg_malloc(l + 1); > +pad = pg_malloc_array(char, l + 1); I can see that tar_get_file_name() has been changed in 0001, which is fine, so I have merged the change from 0002 in dir_get_file_name()@walmethods.c into 0001, for consistency. I don't really have a strong opinion about the rest of 0002, TBH. > 2. I found a small and harmless thinko. The buffer in verify_tar_file() is > actually a char * but for some reason the code did the following: > > buffer = pg_malloc(READ_CHUNK_SIZE * sizeof(uint8)); > > What should we do about it? Just skip the "sizof(uint8)"? This one has already been discussed, see here: https://www.postgresql.org/message-id/aUJ2zxgPCaVsVi2a@ip-10-97-1-34.eu-west-3.compute.internal The story is a bit larger than this single allocation, as it impacts the meaning of the surrounding routines with backup manifests. And applied 0001 after double-checking it. Thanks. -- Michael
Вложения
On 2/27/26 11:00 AM, Michael Paquier wrote: > On Fri, Feb 27, 2026 at 02:15:46AM +0100, Andreas Karlsson wrote: >> 1. What should we do about when we allocate a an array of characters? Would >> it make sense to use pg_array_alloc() or would that jsut be silly? For >> example: >> >> -pad = (char *) pg_malloc(l + 1); >> +pad = pg_malloc_array(char, l + 1); > > I can see that tar_get_file_name() has been changed in 0001, which is > fine, so I have merged the change from 0002 in > dir_get_file_name()@walmethods.c into 0001, for consistency. I don't > really have a strong opinion about the rest of 0002, TBH. Then I think we should skip it. If someone else wants to fix it in the future they are free to do so. >> 2. I found a small and harmless thinko. The buffer in verify_tar_file() is >> actually a char * but for some reason the code did the following: >> >> buffer = pg_malloc(READ_CHUNK_SIZE * sizeof(uint8)); >> >> What should we do about it? Just skip the "sizof(uint8)"? > > This one has already been discussed, see here: > https://www.postgresql.org/message-id/aUJ2zxgPCaVsVi2a@ip-10-97-1-34.eu-west-3.compute.internal > The story is a bit larger than this single allocation, as it impacts > the meaning of the surrounding routines with backup manifests. Thanks for the link! > And applied 0001 after double-checking it. Thanks. Thanks! -- Andreas Karlsson Percona