Thread: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Mahendra Singh Thalor
Date:
Hi,
In commit 643a1a61985bef2590496, we did some cleanup and we replaced if-else with switch case.
Basically, we made a function to open a directory in pg_dumpall. In pg_backup_directory.c file also, we are opening a directory with if-else.
Here, I am attaching a patch which makes both the files similar.
We can move this similar function into one common file also but as of now, I made a static function same as pg_dumpall.c.
Attachment
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Andrew Dunstan
Date:
On 2025-04-07 Mo 8:25 AM, Mahendra Singh Thalor wrote: > Hi, > In commit 643a1a61985bef2590496, we did some cleanup and we replaced > if-else with switch case. > Basically, we made a function to open a directory in pg_dumpall. > In pg_backup_directory.c file also, we are opening a directory with > if-else. > Here, I am attaching a patch which makes both the files similar. > > We can move this similar function into one common file also but as of > now, I made a static function same as pg_dumpall.c. Yeah, let's put it in a common file. There's no point in duplicating it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Mahendra Singh Thalor
Date:
On Mon, 7 Apr 2025 at 18:52, Andrew Dunstan <andrew@dunslane.net> wrote: > > > On 2025-04-07 Mo 8:25 AM, Mahendra Singh Thalor wrote: > > Hi, > > In commit 643a1a61985bef2590496, we did some cleanup and we replaced > > if-else with switch case. > > Basically, we made a function to open a directory in pg_dumpall. > > In pg_backup_directory.c file also, we are opening a directory with > > if-else. > > Here, I am attaching a patch which makes both the files similar. > > > > We can move this similar function into one common file also but as of > > now, I made a static function same as pg_dumpall.c. > > > Yeah, let's put it in a common file. There's no point in duplicating it. Thanks Andrew. I moved this common function dumputils.c file. Here, I am attaching a patch for the same. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
Attachment
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Andrew Dunstan
Date:
On 2025-04-07 Mo 2:59 PM, Mahendra Singh Thalor wrote: > On Mon, 7 Apr 2025 at 18:52, Andrew Dunstan <andrew@dunslane.net> wrote: >> >> On 2025-04-07 Mo 8:25 AM, Mahendra Singh Thalor wrote: >>> Hi, >>> In commit 643a1a61985bef2590496, we did some cleanup and we replaced >>> if-else with switch case. >>> Basically, we made a function to open a directory in pg_dumpall. >>> In pg_backup_directory.c file also, we are opening a directory with >>> if-else. >>> Here, I am attaching a patch which makes both the files similar. >>> >>> We can move this similar function into one common file also but as of >>> now, I made a static function same as pg_dumpall.c. >> >> Yeah, let's put it in a common file. There's no point in duplicating it. > Thanks Andrew. > > I moved this common function dumputils.c file. Here, I am attaching a > patch for the same. > > Pushed with a slight tweaking. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Mahendra Singh Thalor
Date:
On Thu, 10 Apr 2025 at 21:48, Andrew Dunstan <andrew@dunslane.net> wrote: > > > On 2025-04-07 Mo 2:59 PM, Mahendra Singh Thalor wrote: > > On Mon, 7 Apr 2025 at 18:52, Andrew Dunstan <andrew@dunslane.net> wrote: > >> > >> On 2025-04-07 Mo 8:25 AM, Mahendra Singh Thalor wrote: > >>> Hi, > >>> In commit 643a1a61985bef2590496, we did some cleanup and we replaced > >>> if-else with switch case. > >>> Basically, we made a function to open a directory in pg_dumpall. > >>> In pg_backup_directory.c file also, we are opening a directory with > >>> if-else. > >>> Here, I am attaching a patch which makes both the files similar. > >>> > >>> We can move this similar function into one common file also but as of > >>> now, I made a static function same as pg_dumpall.c. > >> > >> Yeah, let's put it in a common file. There's no point in duplicating it. > > Thanks Andrew. > > > > I moved this common function dumputils.c file. Here, I am attaching a > > patch for the same. > > > > > > > Pushed with a slight tweaking. > > > cheers > > > andrew > > -- > Andrew Dunstan > EDB: https://www.enterprisedb.com > Thanks Andrew for committing this. -- Thanks and Regards Mahendra Singh Thalor EnterpriseDB: http://www.enterprisedb.com
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also
From
Álvaro Herrera
Date:
I don't understand why the routine is called "create_or_open_dir". In what sense does this open the directory? I think "check_or_create_dir" would be closer to what this seem to be doing. Is there no TOCTTOU bug in pg_dumpall because of the way this code is written? A malicious user that can create an empty directory that pg_dumpall is going to use as output destination could remove it after the opendir(), then replace it with another directory with a symlink called "global.dat" that causes some other file to be overwritten with the privileges of the user running pg_dumpall. Maybe there's no problem here, but I don't see what the explanation for that is. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/