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.
> if (uic_is_stale(&uic)) {
> - if (adjust_shared_perm(tmp) < 0)
> + if (adjust_shared_perm(get_tempfile_path(f)) < 0)
> goto out;
> - if (rename(tmp, path) < 0)
> + if (rename_tempfile(&f, path) < 0)
> goto out;
> } else {
> - unlink(tmp);
> + delete_tempfile(&f);
> }
> ret = 0;
>
> out:
> if (ret) {
> error_errno("unable to update %s", path);
> - if (uic.cur_fp)
> - fclose(uic.cur_fp);
> - else if (fd >= 0)
> - close(fd);
> - unlink(tmp);
> + if (f)
> + delete_tempfile(&f);
> }
> free(tmp);
> if (uic.old_fp)
