From e081a7c3b7290c50c817598e5f2686d068540762 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 7 Dec 2021 19:26:30 +0100 Subject: usage.c API users: use die_message() for "fatal :" + exit 128 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change code that printed its own "fatal: " message and exited with a status code of 128 to use the die_message() function added in a preceding commit. This change also demonstrates why the return value of die_message_routine() needed to be that of "report_fn". We have callers such as the run-command.c::child_err_spew() which would like to replace its error routine with the return value of "get_die_message_routine()". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/fast-import.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'builtin') diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 20406f6775..2b2e28bad7 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -401,16 +401,18 @@ static void dump_marks(void); static NORETURN void die_nicely(const char *err, va_list params) { + va_list cp; static int zombie; - char message[2 * PATH_MAX]; + report_fn die_message_fn = get_die_message_routine(); - vsnprintf(message, sizeof(message), err, params); - fputs("fatal: ", stderr); - fputs(message, stderr); - fputc('\n', stderr); + va_copy(cp, params); + die_message_fn(err, params); if (!zombie) { + char message[2 * PATH_MAX]; + zombie = 1; + vsnprintf(message, sizeof(message), err, cp); write_crash_report(message); end_packfile(); unkeep_all_packs(); -- cgit v1.3 From adcd4d4c6f9352bef53058161db36bb1ccd5cd9d Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 7 Dec 2021 19:26:31 +0100 Subject: usage.c API users: use die_message() for error() + exit 128 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continue the migration of code that printed a message and exited with 128. In this case the caller used "error()", so we'll be changing the output from "error: " to "fatal: ". This change is intentional and desired. This code is dying, so it should emit "fatal", the only reason it didn't do so was because before the existence of "die_message()" it would have needed to craft its own "fatal: " message. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/notes.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'builtin') diff --git a/builtin/notes.c b/builtin/notes.c index 71c59583a1..2812d1eac4 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -201,11 +201,12 @@ static void prepare_note_data(const struct object_id *object, struct note_data * static void write_note_data(struct note_data *d, struct object_id *oid) { if (write_object_file(d->buf.buf, d->buf.len, blob_type, oid)) { - error(_("unable to write note object")); + int status = die_message(_("unable to write note object")); + if (d->edit_path) - error(_("the note contents have been left in %s"), - d->edit_path); - exit(128); + die_message(_("the note contents have been left in %s"), + d->edit_path); + exit(status); } } -- cgit v1.3 From 0faf84d97d21d5d29132c8c021b16afe72d9fbf3 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 7 Dec 2021 19:26:32 +0100 Subject: gc: return from cmd_gc(), don't call exit() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A minor code cleanup. Let's "return" from cmd_gc() instead of calling exit(). See 338abb0f045 (builtins + test helpers: use return instead of exit() in cmd_*, 2021-06-08) for other such cases. While we're at it add a \n to separate the variable declaration from the rest of the code in this block. Both of these changes make a subsequent change smaller and easier to read. This change isn't really needed for that subsequent change, but now someone viewing that future behavior change won't need to wonder why we're either still calling exit() here, or fixing it while we're at it. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/gc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'builtin') diff --git a/builtin/gc.c b/builtin/gc.c index bcef6a4c8d..900ccfb8d4 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -611,9 +611,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix) } if (detach_auto) { int ret = report_last_gc_error(); + if (ret < 0) /* an I/O error occurred, already reported */ - exit(128); + return 128; if (ret == 1) /* Last gc --auto failed. Skip this one. */ return 0; -- cgit v1.3 From 24f6e6d626b0125f741880918fb3b9b85f607a79 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Tue, 7 Dec 2021 19:26:33 +0100 Subject: usage.c + gc: add and use a die_message_errno() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the "error: " output when we exit with 128 due to gc.log errors to use a "fatal: " prefix instead. To do this add a die_message_errno() a sibling function to the die_errno() added in a preceding commit. Before this we'd expect report_last_gc_error() to return -1 from error_errno() in this case. It already treated a status of 0 and 1 specially. Let's just document that anything that's not 0 or 1 should be returned. We could also retain the "ret < 0" behavior here without hardcoding 128 by returning -128, and having the caller do a "return -ret", but I think this makes more sense, and preserves the path from die_message*()'s return value to the "return" without hardcoding "128". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/gc.c | 13 +++++++------ git-compat-util.h | 1 + usage.c | 12 ++++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) (limited to 'builtin') diff --git a/builtin/gc.c b/builtin/gc.c index 900ccfb8d4..8e60ef1eab 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -470,7 +470,8 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) /* * Returns 0 if there was no previous error and gc can proceed, 1 if * gc should not proceed due to an error in the last run. Prints a - * message and returns -1 if an error occurred while reading gc.log + * message and returns with a non-[01] status code if an error occurred + * while reading gc.log */ static int report_last_gc_error(void) { @@ -484,7 +485,7 @@ static int report_last_gc_error(void) if (errno == ENOENT) goto done; - ret = error_errno(_("cannot stat '%s'"), gc_log_path); + ret = die_message_errno(_("cannot stat '%s'"), gc_log_path); goto done; } @@ -493,7 +494,7 @@ static int report_last_gc_error(void) len = strbuf_read_file(&sb, gc_log_path, 0); if (len < 0) - ret = error_errno(_("cannot read '%s'"), gc_log_path); + ret = die_message_errno(_("cannot read '%s'"), gc_log_path); else if (len > 0) { /* * A previous gc failed. Report the error, and don't @@ -612,12 +613,12 @@ int cmd_gc(int argc, const char **argv, const char *prefix) if (detach_auto) { int ret = report_last_gc_error(); - if (ret < 0) - /* an I/O error occurred, already reported */ - return 128; if (ret == 1) /* Last gc --auto failed. Skip this one. */ return 0; + else if (ret) + /* an I/O error occurred, already reported */ + return ret; if (lock_repo_for_gc(force, &pid)) return 0; diff --git a/git-compat-util.h b/git-compat-util.h index d5e495529c..c6c6f7d6b5 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -480,6 +480,7 @@ NORETURN void usagef(const char *err, ...) __attribute__((format (printf, 1, 2)) NORETURN void die(const char *err, ...) __attribute__((format (printf, 1, 2))); NORETURN void die_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); int die_message(const char *err, ...) __attribute__((format (printf, 1, 2))); +int die_message_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); int error(const char *err, ...) __attribute__((format (printf, 1, 2))); int error_errno(const char *err, ...) __attribute__((format (printf, 1, 2))); void warning(const char *err, ...) __attribute__((format (printf, 1, 2))); diff --git a/usage.c b/usage.c index 76399ba840..3d09e8eea4 100644 --- a/usage.c +++ b/usage.c @@ -233,6 +233,18 @@ int die_message(const char *err, ...) return 128; } +#undef die_message_errno +int die_message_errno(const char *fmt, ...) +{ + char buf[1024]; + va_list params; + + va_start(params, fmt); + die_message_routine(fmt_with_err(buf, sizeof(buf), fmt), params); + va_end(params); + return 128; +} + #undef error_errno int error_errno(const char *fmt, ...) { -- cgit v1.3