On 12/31/2016 07:26 AM, Jeff King wrote: > 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