Re: PATCH: Configurable file mode mask

From: David Steele <david(at)pgmasters(dot)net>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>
Subject: Re: PATCH: Configurable file mode mask
Date: 2018-01-19 19:07:54
Message-ID: 2f9b74a5-effd-5752-bde0-9aae4d6ace91@pgmasters.net
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 1/19/18 3:08 AM, Michael Paquier wrote:
> On Wed, Jan 10, 2018 at 03:19:46PM -0300, Alvaro Herrera wrote:
>> David Steele wrote:
>>> On 1/8/18 8:58 PM, Peter Eisentraut wrote:
>>
>>>> Yeah, I didn't like this aspect when this patch was originally
>>>> submitted. We want to keep the code legible for future new
>>>> contributors. Having these generic-sounding but specific-in-purpose
>>>> wrapper functions can be pretty confusing. Let's use mkdir() when it's
>>>> the appropriate function, and let's figure out a different name for
>>>> "make a data directory subdirectory in a secure and robust way".
>>
>>> How about MakeDirectoryDefaultPerm()? That's what I'll go with if I
>>> don't hear any other ideas. The single call to MakeDirectoryPerm() will
>>> be reverted to mkdir() and I'll remove the function.
>>
>> I'd go with MakeDirectory, documenting exactly what it does and why, and
>> be done with it. If your new function satisfies future users, great; if
>> not, it can be patched (or not) once we know exactly what these callers
>> need.
>
> After going through the thread, I would vote for making things simple by
> just using MakeDirectory() and document precisely what it does. Anyway,
> there is as well the approach of using MakeDirectoryDefaultPerm(), so
> I'll be fine with the decision you make, David. The patchis moved to
> "waiting on author".

I ended up with MakeDirectoryDefaultPerm(), but I'm flexible.

Attached is a new patch set that also adds the following to patch 02.

1) Move permission constants to a new file in common so they can be
shared with the front end.

2) Update all client programs that write to PGDATA: initdb, pg_ctl,
pg_resetwal, pg_rewind, pg_upgrade.

3) Add tests for initdb, pg_ctl, and pg_rewind.

I have yet to add tests for pg_rewindwal and pg_upgrade. pg_rewindwal
doesn't *have* any tests as far as I can tell and pg_upgrade has tests
in a shell script -- it's not clear how I would extend it or reuse the
Perl code for perm testing.

Does anyone have suggestions on tests for pg_resetwal and pg_upgrade?
Should I start from scratch?

Thanks,
--
-David
david(at)pgmasters(dot)net

Attachment Content-Type Size
group-access-v5-01-mkdir.patch text/plain 10.6 KB
group-access-v5-02-group.patch text/plain 42.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2018-01-19 19:10:16 Re: pgsql: Local partitioned indexes
Previous Message Tomas Vondra 2018-01-19 18:59:27 Re: Built-in connection pooling