> -----Original Message----- > From: Jeff King [mailto:p...@peff.net] > Sent: Friday, February 10, 2017 4:15 PM > To: David Turner <david.tur...@twosigma.com> > Cc: git@vger.kernel.org; pclo...@gmail.com; Junio C Hamano > <gits...@pobox.com> > Subject: Re: [PATCH v5] gc: ignore old gc.log files > > > @@ -76,10 +78,30 @@ static void git_config_date_string(const char > > *key, const char **output) static void process_log_file(void) { > > struct stat st; > > - if (!fstat(get_lock_file_fd(&log_lock), &st) && st.st_size) > > + if (fstat(get_lock_file_fd(&log_lock), &st)) { > > + /* > > + * Perhaps there was an i/o error or another > > + * unlikely situation. Try to make a note of > > + * this in gc.log along with any existing > > + * messages. > > + */ > > + FILE *fp; > > + int saved_errno = errno; > > + fp = fdopen(log_lock.tempfile.fd, "a"); > > We usually use xfdopen() to handle (unlikely) errors rather than segfaulting. > But > I think you'd actually want fdopen_lock_file(), which attaches the fd to the > tempfile for flushing and cleanup purposes. > > That said, I'm not sure I understand why you're opening a new stdio > filehandle. > We know that stderr already points to our logfile (that's how content gets > there > in the first place). If there's a problem with the file or the descriptor, > opening a > new filehandle around the same descriptor won't help. > > Speaking of stderr, I wonder if this function should be calling > fflush(stderr) before looking at the fstat result. There could be contents > buffered > there that haven't been written out yet (not from child processes, but perhaps > ones written in this process itself). > Probably unlikely in practice, since stderr is typically unbuffered by > default.
Process_log_file_at_exit calls fflush. Will fix the other.
