Re: PATCH: Configurable file mode mask

On 1/8/18 8:58 PM, Peter Eisentraut wrote:
> On 1/3/18 08:11, Robert Haas wrote:
>> 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.
> 
> 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".

I think there is value to keeping the function names symmetric to the
FileOpen()/FileOpenPerm() variants but I'm clearly in the minority so
there's no point in pursuing it.

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.

Thanks,
-- 
-David
david@pgmasters.net