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(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.
>
> 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(at)pgmasters(dot)net