Re: PATCH: Configurable file mode mask

On Tue, Jan 2, 2018 at 11:43 AM, David Steele <david(at)pgmasters(dot)net> wrote:
>> > I think MakeDirectory() is a good wrapper, but isn't
>> MakeDirectoryPerm() sort of silly?
>
> There's one place in the backend (storage/ipc/ipc.c) that sets non-default
> directory permissions. This function is intended to support that and any
> extensions that need to set custom perms.

Yeah, but all it does is call mkdir(), which could just as well be
called directly. I think there's a pointer to a wrapper when it does
something for you -- supply an argument, log something, handle
portability concerns -- but this wrapper does exactly nothing.

>> + umask(~(stat_buf.st_mode & PG_DIR_MODE_DEFAULT));
>>
>> Hmm, I wonder if this is going to be 100% portable. Maybe some
>> obscure platform won't like an argument with all the high bits set.
>
> Sure - I have masked this with 0777 to clear any high bits. Sound OK?

Seems a little strange to spell it that way when we're using constants
everywhere else. How about writing it like this:

umask(PG_DIR_MODE_DEFAULT & ~stat_buf.st_mode);

I think that reads as "clear all bits from PG_DIR_MODE_DEFAULT that
are not set in stat_buf.st_mode", which sounds like what we want.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company