pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Fix small memory leak in get_dbname_oid_list_from_mfile().
4+ messages / 3 participants
[nested] [flat]
* pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile().
@ 2026-03-15 19:24 Tom Lane <tgl@sss.pgh.pa.us>
2026-03-16 06:29 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Mahendra Singh Thalor <mahi6run@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Tom Lane @ 2026-03-15 19:24 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Fix small memory leak in get_dbname_oid_list_from_mfile().
Coverity complained that this function leaked the dumpdirpath string,
which it did. But we don't need to make a copy at all, because
there's not really any point in trimming trailing slashes from the
directory name here. If that were needed, the initial
file_exists_in_directory() test would have failed, since it doesn't
bother with that (and neither does anyplace else in this file).
Moreover, if we did want that, reimplementing canonicalize_path()
poorly is not the way to proceed. Arguably, all of this code should
be reexamined with an eye to using src/port/path.c's facilities, but
for today I'll settle for getting rid of the memory leak.
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/bb53b8d359d33f10b6274be743c42f6e8ecfbb84
Modified Files
--------------
src/bin/pg_dump/pg_restore.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile().
2026-03-15 19:24 pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Tom Lane <tgl@sss.pgh.pa.us>
@ 2026-03-16 06:29 ` Mahendra Singh Thalor <mahi6run@gmail.com>
2026-03-16 15:48 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Álvaro Herrera <alvherre@kurilemu.de>
0 siblings, 1 reply; 4+ messages in thread
From: Mahendra Singh Thalor @ 2026-03-16 06:29 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
Thanks Tom for fixing this memory leak.
On Mon, 16 Mar 2026 at 00:54, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Fix small memory leak in get_dbname_oid_list_from_mfile().
>
> Coverity complained that this function leaked the dumpdirpath string,
> which it did. But we don't need to make a copy at all, because
> there's not really any point in trimming trailing slashes from the
> directory name here. If that were needed, the initial
> file_exists_in_directory() test would have failed, since it doesn't
> bother with that (and neither does anyplace else in this file).
> Moreover, if we did want that, reimplementing canonicalize_path()
> poorly is not the way to proceed. Arguably, all of this code should
> be reexamined with an eye to using src/port/path.c's facilities, but
> for today I'll settle for getting rid of the memory leak.
>
> Branch
> ------
> master
>
> Details
> -------
>
https://git.postgresql.org/pg/commitdiff/bb53b8d359d33f10b6274be743c42f6e8ecfbb84
>
> Modified Files
> --------------
> src/bin/pg_dump/pg_restore.c | 16 +++-------------
> 1 file changed, 3 insertions(+), 13 deletions(-)
>
We were trimming slashes due to the below test case.
*Ex:*
./pg_dumpall -f dumpdir --format=d
./pg_restore *dumpdir/ *-d postgres -C --format=d --verbose
pg_restore: found database "template1" (OID: 1) in file "*dumpdir//map.dat*"
pg_restore: found database "postgres" (OID: 5) in file "*dumpdir//map.dat*"
pg_restore: found 2 database names in "map.dat"
pg_restore: need to restore 2 databases out of 2 databases
Here, before map.dat, there were 2 slashes so we were trimming slashes.
Please add your opinion for this output.
--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile().
2026-03-15 19:24 pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Tom Lane <tgl@sss.pgh.pa.us>
2026-03-16 06:29 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Mahendra Singh Thalor <mahi6run@gmail.com>
@ 2026-03-16 15:48 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-03-16 16:47 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 4+ messages in thread
From: Álvaro Herrera @ 2026-03-16 15:48 UTC (permalink / raw)
To: Mahendra Singh Thalor <mahi6run@gmail.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-committers@lists.postgresql.org
On 2026-Mar-16, Mahendra Singh Thalor wrote:
> We were trimming slashes due to the below test case.
>
> *Ex:*
> ./pg_dumpall -f dumpdir --format=d
>
> ./pg_restore *dumpdir/ *-d postgres -C --format=d --verbose
> pg_restore: found database "template1" (OID: 1) in file "*dumpdir//map.dat*"
> pg_restore: found database "postgres" (OID: 5) in file "*dumpdir//map.dat*"
> pg_restore: found 2 database names in "map.dat"
> pg_restore: need to restore 2 databases out of 2 databases
>
> Here, before map.dat, there were 2 slashes so we were trimming slashes.
> Please add your opinion for this output.
I think this is perfectly fine. Two slashes work correctly, and many
other tools behave this way when given redundant slashes. If the user
doesn't want to see the redundant slashes, they can just not specify a
trailing slash in the argument value.
--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"¿Cómo puedes confiar en algo que pagas y que no ves,
y no confiar en algo que te dan y te lo muestran?" (Germán Poo)
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile().
2026-03-15 19:24 pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Tom Lane <tgl@sss.pgh.pa.us>
2026-03-16 06:29 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Mahendra Singh Thalor <mahi6run@gmail.com>
2026-03-16 15:48 ` Re: pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Álvaro Herrera <alvherre@kurilemu.de>
@ 2026-03-16 16:47 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 4+ messages in thread
From: Tom Lane @ 2026-03-16 16:47 UTC (permalink / raw)
To: Álvaro Herrera <alvherre@kurilemu.de>; +Cc: Mahendra Singh Thalor <mahi6run@gmail.com>; pgsql-committers@lists.postgresql.org
=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:
> On 2026-Mar-16, Mahendra Singh Thalor wrote:
>> We were trimming slashes due to the below test case.
>>
>> *Ex:*
>> ./pg_dumpall -f dumpdir --format=d
>>
>> ./pg_restore *dumpdir/ *-d postgres -C --format=d --verbose
>> pg_restore: found database "template1" (OID: 1) in file "*dumpdir//map.dat*"
>> pg_restore: found database "postgres" (OID: 5) in file "*dumpdir//map.dat*"
>> pg_restore: found 2 database names in "map.dat"
>> pg_restore: need to restore 2 databases out of 2 databases
>>
>> Here, before map.dat, there were 2 slashes so we were trimming slashes.
>> Please add your opinion for this output.
> I think this is perfectly fine. Two slashes work correctly, and many
> other tools behave this way when given redundant slashes. If the user
> doesn't want to see the redundant slashes, they can just not specify a
> trailing slash in the argument value.
As I said in my commit message, if we wanted to do something about
this the correct thing would be to apply canonicalize_path to the
constructed strings. On Unix systems that's really just cosmetic, as
Alvaro says. But if memory serves it's not necessarily so on Windows.
Has anyone tested this patch on Windows, with Windows peculiarities
like using backslash separators in the given path?
regards, tom lane
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-03-16 16:47 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-03-15 19:24 pgsql: Fix small memory leak in get_dbname_oid_list_from_mfile(). Tom Lane <tgl@sss.pgh.pa.us>
2026-03-16 06:29 ` Mahendra Singh Thalor <mahi6run@gmail.com>
2026-03-16 15:48 ` Álvaro Herrera <alvherre@kurilemu.de>
2026-03-16 16:47 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox