On 10/23/2014 07:17 PM, Jakub Hrozek wrote:
On Thu, Oct 23, 2014 at 07:12:44PM +0200, Michal Židek wrote:
> On 10/23/2014 07:02 PM, Jakub Hrozek wrote:
>> On Thu, Oct 23, 2014 at 05:30:56PM +0200, Pavel Reichl wrote:
>>> - ret = create_pipe_fd(ctx->sock_name, &ctx->fd, 0111);
>>> + umask(0111);
>>> + ret = create_pipe_fd(ctx->sock_name, &ctx->fd);
>>> + umask(DEF_RSP_UMASK);
>>> assert_int_equal(ret, EOK);
>>> assert_int_not_equal(ctx->fd, -1);
>>> check_sock_properties(ctx, 0666);
>>>
>>> /* Make sure we can overwrite an existing socket */
>>> - ret = create_pipe_fd(ctx->sock_name, &ctx->fd, 0000);
>>> + umask(0000);
>>> + ret = create_pipe_fd(ctx->sock_name, &ctx->fd);
>>> + umask(DEF_RSP_UMASK);
>>
>> I don't like a pattern like this that requires the caller to set and
>> reset the caller. I would prefer if we still passed either the required
>> mode or umask to the create_pipe_fd(), then create_pipe_fd() would first
>> save the old umask, set the new one, create the pipes and then reset the
>> old one in the "done" handler.
>>
>> The default umask should be set in a very generic function like
>> server_setup (maybe we already do that, not sure)
>
> It can not be done in server_setup (it is too late). So we can
> have a new "setup" like function or we just call umask after
> the process is created (in main) with some #defined value.
If the new function called just umask then I don't see a reason for the
wrapper -- maybe for future extensibility?
I think calling umask will be just fine. No new function needed,
I should not have mentioned it probably.
But I'm fine with both these options, I just don't like us setting an umask
in create_pipe_fd without resetting in back in the same function. A
utility function should have no side effects.