lockfile: report when rollback fails

We do not report to the caller when rolling back a lockfile fails, which will be needed by the reftable compaction logic in a subsequent commit. It also cannot really report on all errors because the function calls `delete_tempfile()`, which doesn't return an error either. Refactor the code so that both `delete_tempfile()` and `rollback_lock_file()` return an error code. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Mar 7, 2024 at 14:10 UTC 4ae540d421c5a763a14fbe79a35d6f6ca004a21b
3 files changed +17 -12
lockfile.h
+3 -3
@@ -321,11 +321,11 @@ static inline int commit_lock_file_to(struct lock_file *lk, const char *path)
321 * Roll back `lk`: close the file descriptor and/or file pointer and
322 * remove the lockfile. It is a NOOP to call `rollback_lock_file()`
323 * for a `lock_file` object that has already been committed or rolled
324 - * back.
324 + * back. No error will be returned in this case.
325 */
326 -static inline void rollback_lock_file(struct lock_file *lk)
326 +static inline int rollback_lock_file(struct lock_file *lk)
327 {
328 - delete_tempfile(&lk->tempfile);
328 + return delete_tempfile(&lk->tempfile);
329 }
330
331 #endif /* LOCKFILE_H */
tempfile.c
+13 -8
@@ -50,15 +50,17 @@
50
51 static VOLATILE_LIST_HEAD(tempfile_list);
52
53 -static void remove_template_directory(struct tempfile *tempfile,
53 +static int remove_template_directory(struct tempfile *tempfile,
54 int in_signal_handler)
55 {
56 if (tempfile->directory) {
57 if (in_signal_handler)
58 - rmdir(tempfile->directory);
58 + return rmdir(tempfile->directory);
59 else
60 - rmdir_or_warn(tempfile->directory);
60 + return rmdir_or_warn(tempfile->directory);
61 }
62 +
63 + return 0;
64 }
65
66 static void remove_tempfiles(int in_signal_handler)
@@ -353,16 +355,19 @@ int rename_tempfile(struct tempfile **tempfile_p, const char *path)
355 return 0;
356 }
357
356 -void delete_tempfile(struct tempfile **tempfile_p)
358 +int delete_tempfile(struct tempfile **tempfile_p)
359 {
360 struct tempfile *tempfile = *tempfile_p;
361 + int err = 0;
362
363 if (!is_tempfile_active(tempfile))
361 - return;
364 + return 0;
365
363 - close_tempfile_gently(tempfile);
364 - unlink_or_warn(tempfile->filename.buf);
365 - remove_template_directory(tempfile, 0);
366 + err |= close_tempfile_gently(tempfile);
367 + err |= unlink_or_warn(tempfile->filename.buf);
368 + err |= remove_template_directory(tempfile, 0);
369 deactivate_tempfile(tempfile);
370 *tempfile_p = NULL;
371 +
372 + return err ? -1 : 0;
373 }
tempfile.h
+1 -1
@@ -269,7 +269,7 @@ int reopen_tempfile(struct tempfile *tempfile);
269 * `delete_tempfile()` for a `tempfile` object that has already been
270 * deleted or renamed.
271 */
272 -void delete_tempfile(struct tempfile **tempfile_p);
272 +int delete_tempfile(struct tempfile **tempfile_p);
273
274 /*
275 * Close the file descriptor and/or file pointer if they are still