Thread

1. Avoid resource leak (contrib/postgres_fdw/connection.c)

   Ranier Vilela <ranier.vf@gmail.com> — 2026-03-16T11:45:49Z

   Hi.
   
   Per Coverity.
   
   CID 1645716: (#1 of 1): Resource leak (RESOURCE_LEAK)
   8. leaked_storage: Variable str going out of scope leaks the storage
   str.data points to.
   
   The function *postgres_fdw_connection* leaks the contents of
   var str.data
   Once that function *cstring_to_text* palloc the contents
   must be necessary to free the var str.data.
   
   patch attached.
   
   best regards,
   Ranier Vilelas
   

2. Re: Avoid resource leak (contrib/postgres_fdw/connection.c)

   Matheus Alcantara <matheusssilv97@gmail.com> — 2026-03-16T14:16:30Z

   On 16/03/26 08:45, Ranier Vilela wrote:
   > Hi.
   > 
   > Per Coverity.
   > 
   > CID 1645716: (#1 of 1): Resource leak (RESOURCE_LEAK)
   > 8. leaked_storage: Variable str going out of scope leaks the storage
   > str.data points to.
   > 
   > The function *postgres_fdw_connection* leaks the contents of
   > var str.data
   > Once that function *cstring_to_text* palloc the contents
   > must be necessary to free the var str.data.
   > 
   > patch attached.
   > 
   
   Hi,
   
   Thanks for the patch, it looks correct to me. I've searched for this 
   pattern `PG_RETURN_TEXT_P(cstring_to_text(.*.data` in other places and 
   I've just found on postgres_fdw/connection.c
   
   I've also search for other cases of `return cstring_to_text(...)` 
   usages and I didn't found anything that seems suspicious.
   
   Tests are also passing.
   
   --
   Matheus Alcantara
   EDB: https://www.enterprisedb.com
   
   
   
   

3. Re: Avoid resource leak (contrib/postgres_fdw/connection.c)

   Ranier Vilela <ranier.vf@gmail.com> — 2026-03-16T14:18:54Z

   Hi.
   
   Em seg., 16 de mar. de 2026 às 11:16, Matheus Alcantara <
   matheusssilv97@gmail.com> escreveu:
   
   > On 16/03/26 08:45, Ranier Vilela wrote:
   > > Hi.
   > >
   > > Per Coverity.
   > >
   > > CID 1645716: (#1 of 1): Resource leak (RESOURCE_LEAK)
   > > 8. leaked_storage: Variable str going out of scope leaks the storage
   > > str.data points to.
   > >
   > > The function *postgres_fdw_connection* leaks the contents of
   > > var str.data
   > > Once that function *cstring_to_text* palloc the contents
   > > must be necessary to free the var str.data.
   > >
   > > patch attached.
   > >
   >
   > Hi,
   >
   > Thanks for the patch, it looks correct to me. I've searched for this
   > pattern `PG_RETURN_TEXT_P(cstring_to_text(.*.data` in other places and
   > I've just found on postgres_fdw/connection.c
   >
   > I've also search for other cases of `return cstring_to_text(...)`
   > usages and I didn't found anything that seems suspicious.
   >
   > Tests are also passing.
   >
   Thanks for taking a look.
   
   best regards,
   Ranier Vilela
   

4. Re: Avoid resource leak (contrib/postgres_fdw/connection.c)

   Fujii Masao <masao.fujii@gmail.com> — 2026-03-16T23:07:47Z

   On Mon, Mar 16, 2026 at 8:46 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
   >
   > Hi.
   >
   > Per Coverity.
   >
   > CID 1645716: (#1 of 1): Resource leak (RESOURCE_LEAK)
   > 8. leaked_storage: Variable str going out of scope leaks the storage str.data points to.
   >
   > The function *postgres_fdw_connection* leaks the contents of
   > var str.data
   > Once that function *cstring_to_text* palloc the contents
   > must be necessary to free the var str.data.
   
   It seems that postgres_fdw_connection() is expected to be called by
   ForeignServerConnectionString() via OidFunctionCall3(),
   and to allocate memory (including str.data) in the FDWConnectionContext
   memory context created by ForeignServerConnectionString().
   
   After calling postgres_fdw_connection(), ForeignServerConnectionString()
   deletes FDWConnectionContext. So it seems to me that any memory
   allocated in that context, including str.data, would not leak. No?
   
   Regards,
   
   -- 
   Fujii Masao
   
   
   
   

5. Re: Avoid resource leak (contrib/postgres_fdw/connection.c)

   Matheus Alcantara <matheusssilv97@gmail.com> — 2026-03-18T17:42:35Z

   On 16/03/26 20:07, Fujii Masao wrote:
   > On Mon, Mar 16, 2026 at 8:46 PM Ranier Vilela <ranier.vf@gmail.com> wrote:
   >>
   >> Hi.
   >>
   >> Per Coverity.
   >>
   >> CID 1645716: (#1 of 1): Resource leak (RESOURCE_LEAK)
   >> 8. leaked_storage: Variable str going out of scope leaks the storage str.data points to.
   >>
   >> The function *postgres_fdw_connection* leaks the contents of
   >> var str.data
   >> Once that function *cstring_to_text* palloc the contents
   >> must be necessary to free the var str.data.
   > 
   > It seems that postgres_fdw_connection() is expected to be called by
   > ForeignServerConnectionString() via OidFunctionCall3(),
   > and to allocate memory (including str.data) in the FDWConnectionContext
   > memory context created by ForeignServerConnectionString().
   > 
   > After calling postgres_fdw_connection(), ForeignServerConnectionString()
   > deletes FDWConnectionContext. So it seems to me that any memory
   > allocated in that context, including str.data, would not leak. No?
   > 
   
   Yeah, this sounds correct, but I'm wondering if the current pfree 
   calls on postgres_fdw_connection() is needed?
   
   My previous review was assuming that any allocated memory on 
   postgres_fdw_connection() should be free at the end because of these 
   calls, but ForeignServerConnectionString() already assume that this 
   function leak memory:
   	/*
   	 * GetForeignServer, GetForeignDataWrapper, and the connection function
   	 * itself all leak memory into CurrentMemoryContext. Switch to a temporary
   	 * context for easy cleanup.
   	 */
   	tempContext = AllocSetContextCreate(CurrentMemoryContext,
   										"FDWConnectionContext",
   										ALLOCSET_SMALL_SIZES);
   
   --
   Matheus Alcantara
   EDB: https://www.enterprisedb.com