Re: [PATCH v5] gc: ignore old gc.log files
From: Jeff King <hidden>
Date: 2017-02-10 21:15:17
quoted hunk ↗ jump to hunk
@@ -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. -Peff