Re: PATCH: Configurable file mode mask

On Tue, Jan 2, 2018 at 11:43 AM, David Steele <david@pgmasters.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