as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

Started by Mahendra Singh Thalorabout 1 year ago6 messageshackers
Jump to latest
#1Mahendra Singh Thalor
mahi6run@gmail.com

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.

--
Thanks and Regards
Mahendra Singh Thalor
EnterpriseDB: http://www.enterprisedb.com

Attachments:

v01_move-create-dir-code-to-the-switch-in-pg_backup_dir.patchapplication/octet-stream; name=v01_move-create-dir-code-to-the-switch-in-pg_backup_dir.patchDownload+37-35
#2Andrew Dunstan
andrew@dunslane.net
In reply to: Mahendra Singh Thalor (#1)
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

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

#3Mahendra Singh Thalor
mahi6run@gmail.com
In reply to: Andrew Dunstan (#2)
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

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

Attachments:

v02_move-create-dir-code-to-the-switch-in-pg_backup_dir.patchapplication/octet-stream; name=v02_move-create-dir-code-to-the-switch-in-pg_backup_dir.patchDownload+39-72
#4Andrew Dunstan
andrew@dunslane.net
In reply to: Mahendra Singh Thalor (#3)
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

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

#5Mahendra Singh Thalor
mahi6run@gmail.com
In reply to: Andrew Dunstan (#4)
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

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

#6Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Mahendra Singh Thalor (#3)
Re: as per commit 643a1a61985bef2590496, move create/open dir code to function using switch case of pg_backup_directory.c file also

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/