Re: [PATCH v3 11/23] log_ref_setup(): separate code for create vs non-create

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Dec 31, 2016, 07:52 UTC
Message-ID: <2bd120b1-377a-0416-1591-1da4ab69d655@alum.mit.edu>
In-Reply-To: <20161231062607.uxftwujophv37ymb@sigill.intra.peff.net>

On 12/31/2016 07:26 AM, Jeff King wrote:

Show 13 quoted lines

> On Sat, Dec 31, 2016 at 04:12:51AM +0100, Michael Haggerty wrote:
>> [...]
>> -	adjust_shared_perm(logfile->buf);
>> -	close(logfd);
>> +	if (logfd >= 0) {
>> +		adjust_shared_perm(logfile->buf);
>> +		close(logfd);
>> +	}
>> +
> 
> Hmm. I would have thought in the existing-logfile case that we would not
> need to adjust_shared_perm(). But maybe we just do it anyway to pick up
> potentially-changed config.

I didn't change this aspect of the code's behavior (though I also found it a bit surprising).

Another thing I considered was changing adjust_shared_perm() to adjust the file's permissions through the open file descriptor using fchmod(). But that function has a bunch of callers, and I didn't want to have to duplicate the code, nor did I have the energy to change all of its callers (if that would even make sense for all callers, which I doubt).

> [...]

Michael