@matheusbsbs / matheus-bagliano / commits / 45c6b1ed24

always check return value of close_tempfile

If close_tempfile() encounters an error, then it deletes the tempfile and resets the "struct tempfile". But many code paths ignore the return value and continue to use the tempfile. Instead, we should generally treat this the same as a write() error. Note that in the postimage of some of these cases our error message will be bogus after a failed close because we look at tempfile->filename (either directly or via get_tempfile_path). But after the failed close resets the tempfile object, this is guaranteed to be the empty string. That will be addressed in a future patch (because there are many more cases of the same problem than just these instances). Note also in the hunk in gpg-interface.c that it's fine to call delete_tempfile() in the error path, even if close_tempfile() failed and already deleted the file. The tempfile code is smart enough to know the second deletion is a noop. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Sep 5, 2017 at 08:14 UTC 45c6b1ed24724f7f3041a60a4313df7d9c4b9909

3 files changed +6 -6

diff.c

+2 -2

@@ -3738,9 +3738,9 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp,

3738 blob = buf.buf;

3739 size = buf.len;

3740 }

3741 - if (write_in_full(fd, blob, size) != size)

3741 + if (write_in_full(fd, blob, size) != size ||

3742 + close_tempfile(&temp->tempfile))

3743 die_errno("unable to write temp-file");

3743 - close_tempfile(&temp->tempfile);

3744 temp->name = get_tempfile_path(&temp->tempfile);

3745 oid_to_hex_r(temp->hex, oid);

3746 xsnprintf(temp->mode, sizeof(temp->mode), "%06o", mode);

gpg-interface.c

+2 -2

@@ -209,13 +209,13 @@ int verify_signed_buffer(const char *payload, size_t payload_size,

209 fd = mks_tempfile_t(&temp, ".git_vtag_tmpXXXXXX");

210 if (fd < 0)

211 return error_errno(_("could not create temporary file"));

212 - if (write_in_full(fd, signature, signature_size) < 0) {

212 + if (write_in_full(fd, signature, signature_size) < 0 ||

213 + close_tempfile(&temp) < 0) {

214 error_errno(_("failed writing detached signature to '%s'"),

215 temp.filename.buf);

216 delete_tempfile(&temp);

217 return -1;

218 }

218 - close_tempfile(&temp);

219

220 argv_array_pushl(&gpg.args,

221 gpg_program,

shallow.c

+2 -2

@@ -295,10 +295,10 @@ const char *setup_temporary_shallow(const struct oid_array *extra)

295 if (write_shallow_commits(&sb, 0, extra)) {

296 fd = xmks_tempfile(&temp, git_path("shallow_XXXXXX"));

297

298 - if (write_in_full(fd, sb.buf, sb.len) != sb.len)

298 + if (write_in_full(fd, sb.buf, sb.len) != sb.len ||

299 + close_tempfile(&temp) < 0)

300 die_errno("failed to write to %s",

301 get_tempfile_path(&temp));

301 - close_tempfile(&temp);

302 strbuf_release(&sb);

303 return get_tempfile_path(&temp);

304 }