On Fri, Jun 07, 2024 at 09:02:14AM -0700, Junio C Hamano wrote: > Taylor Blau <me@xxxxxxxxxxxx> writes: > > > @@ -121,27 +120,22 @@ static int update_info_file(char *path, > > } > > > > uic.cur_fp = NULL; > > - if (fclose(to_close)) > > - goto out; > > We should fflush() of cur_fp before nuking it, at least, no? > > In the original code, to_close was a mere copy of uic.cur_fp and we > made sure that anything buffered at the stdio layer are flushed to > the underlying file desciptor (fd that we obtained from > git_mkstemp_mode() in the original code) with this fclose() call. > > We no longer do so. We later call rename_tempfile() to close the > underlying file descriptor and move the temporary file to its final > place, but I do not see what guarantee we have that we do not lose > what we had buffered in the stdio with the updated code. rename_tempfile() first calls close_tempfile_gently() before calling rename(). close_tempfile_gently() calls fclose() on temp->fp before returns, so we get our fflush() call implicitly there. IOW, the tempfile.h API is happy for us to call rename_tempfile() without having explicitly closed or flushed the underlying file pointer. Thanks, Taylor
