Exit code and logging invariants for log_fatal and hardware log_error

AGENTS.md

@@ -124,6 +124,9 @@ SnapRAID is designed to recover from an interruption, including `SIGKILL`, at an
   Or use callback iteration helpers: `tommy_list_foreach()`, `tommy_list_foreach_arg()`, `tommy_hashdyn_foreach()`, etc.
 - **Cleanup**: Use `goto` pattern for cleanup in complex functions when appropriate
 - **Fatal paths**: Use `exit(EXIT_FAILURE)` for unrecoverable errors; `os_abort()` for internal inconsistencies
+- **Exit Code & Logging Invariants**:
+  - `log_fatal()`: Any invocation of `log_fatal()` represents an unrecoverable failure and must force a non-zero exit code upon termination.
+  - `log_error()`: Whenever `log_error()` is called with a hardware error (`is_hw_errno(err)`), it must force a non-zero exit code upon termination.
 
 #### Threading
 

cmdline/app.h

@@ -13,8 +13,8 @@
 
 #define STATIC_ASSERT(name, cond) typedef char static_assert_ ## name[(cond) ? 1 : -1]
 
+#define EDATA (-1) /**< Silent data corruption. */
 #define EINTERNAL EFAULT /**< Internal assertion failed. */
-#define EDATA EIO /**< Silent data corruption. */
 #define ESOFT EINVAL /**< Software error, like permission denied. */
 #define EUSER EINVAL /**< Invalid value specified by the user. */
 #define EEXTERNAL EINVAL /**< Invalid external interface behaviour. */

cmdline/check.c

@@ -3230,6 +3230,11 @@ int state_check(struct snapraid_state* state, int fix, block_off_t blockstart, b
 	if (process_error != 0)
 		return -1;
 
+	if (log_hardware_errors() != 0) {
+		msg_status("Everything done, but hardware errors were encountered in the process\n");
+		return -1;
+	}
+
 	msg_status("Everything OK\n");
 
 	return 0;

cmdline/dry.c

@@ -491,6 +491,11 @@ int state_dry(struct snapraid_state* state, block_off_t blockstart, block_off_t
 	if (process_error != 0)
 		return -1;
 
+	if (log_hardware_errors() != 0) {
+		msg_status("Everything done, but hardware errors were encountered in the process\n");
+		return -1;
+	}
+
 	msg_status("Everything OK\n");
 
 	return 0;

cmdline/parity.c

@@ -430,8 +430,14 @@ static int parity_handle_fill(struct snapraid_split_handle* split, data_off_t si
 
 		/* note that in Windows ftruncate is really allocating space */
 		if (ftruncate(spaceholder_f, WINDOWS_SPACEHOLDER_SIZE) != 0) {
-			log_fatal(errno, "WARNING Failed to resize the space holder file '%s' to %u bytes.\n", spaceholder_path, WINDOWS_SPACEHOLDER_SIZE);
-			log_fatal(errno, "Assuming that no more space is available.\n");
+			if (is_hw(errno)) {
+				log_fatal(errno, "Hardware error while resizing space holder file '%s'. %s.\n", spaceholder_path, strerror(errno));
+				close(spaceholder_f);
+				remove(spaceholder_path);
+				return -1;
+			}
+			log_info(0, "WARNING Failed to resize the space holder file '%s' to %u bytes. %s.\n", spaceholder_path, WINDOWS_SPACEHOLDER_SIZE, strerror(errno));
+			log_info(0, "Assuming that no more space is available.\n");
 			close(spaceholder_f);
 			remove(spaceholder_path);
 			return 0;
@@ -505,8 +511,13 @@ static int parity_handle_fill(struct snapraid_split_handle* split, data_off_t si
 #ifdef _WIN32
 	/* now delete the spaceholder file */
 	if (remove(spaceholder_path) != 0) {
-		log_error(errno, "WARNING Failed to remove the space holder file '%s'.\n", spaceholder_path);
-		log_error(errno, "Continuing anyway.\n");
+		int err = errno;
+
+		log_error(err, "WARNING Failed to remove the space holder file '%s'. %s.\n", spaceholder_path, strerror(err));
+		if (is_hw(err)) {
+			return -1;
+		}
+		log_error(err, "Continuing anyway.\n");
 	}
 #endif
 

cmdline/scrub.c

@@ -1264,6 +1264,11 @@ int state_scrub(struct snapraid_state* state, int plan100, int olderthan)
 	if (process_error != 0)
 		return -1;
 
+	if (log_hardware_errors() != 0) {
+		msg_status("Everything done, but hardware errors were encountered in the process\n");
+		return -1;
+	}
+
 	msg_status("Everything OK\n");
 
 	return 0;

cmdline/snapraid.c

@@ -1503,11 +1503,15 @@ int snapraid_main(int argc, char* argv[])
 			state_locate_mark_tail_blocks_for_resync(&state, opt.parity_tail);
 
 		if (opt.gui_touch_before) {
-			state_touch(&state);
+			ret = state_touch(&state);
 
 			/* save the new state if required */
 			if (state.need_write)
 				state_write(&state);
+
+			if (ret != 0) {
+				exit(EXIT_FAILURE);
+			}
 		}
 
 		ret = state_snapshot_scan(&state);
@@ -1652,7 +1656,7 @@ int snapraid_main(int argc, char* argv[])
 
 		memory();
 
-		state_touch(&state);
+		ret = state_touch(&state);
 
 		/*
 		 * Intercept signals while operating
@@ -1779,6 +1783,10 @@ int snapraid_main(int argc, char* argv[])
 		/* LCOV_EXCL_STOP */
 	}
 
+	/* force failure if hardware errors were encountered */
+	if (ret == 0 && log_hardware_errors() != 0)
+		ret = -1;
+
 	/* close log file */
 	log_close(log_file);
 

cmdline/state.c

@@ -353,7 +353,7 @@ static void state_config_check(struct snapraid_state* state, const char* path, t
 						 * because we want to use these disks
 						 */
 						if (!state->opt.no_warnings)
-							log_fatal(EUSER, "DANGER! Ignoring that disks '%s' and '%s' are on the same device\n", disk->name, other->name);
+							log_error(EUSER, "DANGER! Ignoring that disks '%s' and '%s' are on the same device\n", disk->name, other->name);
 					} else {
 						/* LCOV_EXCL_START */
 						log_fatal(ESOFT, "Disks '%s' and '%s' are on the same device.\n", disk->mount_point, other->mount_point);
@@ -383,7 +383,7 @@ static void state_config_check(struct snapraid_state* state, const char* path, t
 								 * because we want to use these disks
 								 */
 								if (!state->opt.no_warnings)
-									log_fatal(EUSER, "DANGER! Ignoring that disks '%s' and %s '%s' are on the same device\n", disk->mount_point, lev_name(l), state->parity[l].split_map[s].path);
+									log_error(EUSER, "DANGER! Ignoring that disks '%s' and %s '%s' are on the same device\n", disk->mount_point, lev_name(l), state->parity[l].split_map[s].path);
 							} else {
 								/* LCOV_EXCL_START */
 								log_fatal(ESOFT, "Disk '%s' and %s '%s' are on the same device.\n", disk->mount_point, lev_name(l), state->parity[l].split_map[s].path);
@@ -446,7 +446,7 @@ static void state_config_check(struct snapraid_state* state, const char* path, t
 								 * because we want to use these disks
 								 */
 								if (!state->opt.no_warnings)
-									log_fatal(EUSER, "DANGER! Skipping parities '%s' and '%s' on the same device\n", lev_config_name(l), lev_config_name(j));
+									log_error(EUSER, "DANGER! Skipping parities '%s' and '%s' on the same device\n", lev_config_name(l), lev_config_name(j));
 							} else {
 								/* LCOV_EXCL_START */
 								log_fatal(ESOFT, "Parity '%s' and '%s' are on the same device.\n", state->parity[l].split_map[s].path, state->parity[j].split_map[t].path);
@@ -875,7 +875,7 @@ void state_config(struct snapraid_state* state, const char* path, const char* co
 							dev = 0;
 							*uuid = 0;
 							skip_access = 1;
-							log_fatal(errno, "DANGER! Skipping inaccessible parity disk '%s'...\n", tag);
+							log_error(errno, "DANGER! Skipping inaccessible parity disk '%s'...\n", tag);
 						} else {
 							/* LCOV_EXCL_START */
 							log_fatal(errno, "Error accessing 'parity' dir '%s' specification in '%s' at line %u\n", device, path, line);
@@ -1019,7 +1019,7 @@ void state_config(struct snapraid_state* state, const char* path, const char* co
 				if (state->opt.skip_content_access) {
 					/* use a fake device */
 					dev = 0;
-					log_fatal(errno, "WARNING! Skipping inaccessible content file '%s'...\n", buffer);
+					log_error(errno, "WARNING! Skipping inaccessible content file '%s'...\n", buffer);
 				} else {
 					/* LCOV_EXCL_START */
 					log_fatal(errno, "Error accessing 'content' dir '%s' specification in '%s' at line %u\n", device, path, line);
@@ -1139,7 +1139,7 @@ void state_config(struct snapraid_state* state, const char* path, const char* co
 						dev = 0;
 						*uuid = 0;
 						skip_access = 1;
-						log_fatal(errno, "DANGER! Skipping inaccessible data disk '%s'...\n", buffer);
+						log_error(errno, "DANGER! Skipping inaccessible data disk '%s'...\n", buffer);
 					} else {
 						/* LCOV_EXCL_START */
 						log_fatal(errno, "Error accessing 'disk' '%s' specification in '%s' at line %u\n", dir, device, line);
@@ -6387,7 +6387,7 @@ int state_progress(struct snapraid_state* state, struct snapraid_io* io, block_o
 			msg_bar("\n");
 			msg_flush();
 		}
-		log_fatal(EUSER, "Stopping for interruption at block %" PRIu64 "\n", blockpos);
+		log_error(EUSER, "Stopping for interruption at block %" PRIu64 "\n", blockpos);
 		log_tag("sigint:%" PRIu64 ": SIGINT received\n", blockpos);
 		log_flush();
 		return 1;

cmdline/state.h

@@ -263,7 +263,7 @@ void state_scan(struct snapraid_state* state);
 /**
  * Set the nanosecond timestamp of all files that have a zero value.
  */
-void state_touch(struct snapraid_state* state);
+int state_touch(struct snapraid_state* state);
 
 /**
  * Map devices if not already done.

cmdline/support.c

@@ -181,6 +181,8 @@ uint64_t random_u64(void)
 int is_hw(int err)
 {
 	/* LCOV_EXCL_START */
+	if (err == EDATA)
+		return 1;
 	if (err == EIO)
 		return 1;
 #ifdef EFSCORRUPTED
@@ -211,6 +213,19 @@ int is_hw(int err)
 	return 0;
 }
 
+/**
+ * Return if the error is a hardware error represented by a system errno.
+ *
+ * Unlike is_hw(), this excludes synthetic errors such as EDATA, which
+ * may be reported while still allowing a zero process exit code.
+ * 
+ * All other hardware errors must prevent a successful process exit.
+ */
+int is_hw_errno(int err)
+{
+	return err > 0 && is_hw(err);
+}
+
 void log_fatal_errno(int err, const char* name)
 {
 	/* LCOV_EXCL_START */
@@ -297,6 +312,18 @@ int msg_fatal_stderr = 1;
 FILE* stdlog = 0;
 int msg_line_prev = 0; /**< Previous line width on stdout */
 int msg_line_curr = 0; /**< Current line width on stdout */
+static unsigned log_hardware_error_count = 0;
+
+unsigned log_hardware_errors(void)
+{
+	unsigned count;
+
+	lock_msg();
+	count = log_hardware_error_count;
+	unlock_msg();
+
+	return count;
+}
 
 /*
  * Note that in the following functions we always flush both
@@ -379,12 +406,14 @@ void vlog_fatal(int err, const char* format, va_list ap, const char* post)
 
 	lock_msg();
 
+	if (is_hw(err))
+		++log_hardware_error_count;
+
 	if (stdlog) {
 		if (is_hw(err))
 			log_failed = fprintf(stdlog, "msg:fatal_hardware: ") < 0;
 		else
 			log_failed = fprintf(stdlog, "msg:fatal: ") < 0;
-
 		va_list ap_copy;
 		va_copy(ap_copy, ap);
 		if (vfprintf(stdlog, format, ap_copy) < 0)
@@ -427,6 +456,9 @@ void vlog_error(int err, const char* format, va_list ap, const char* post)
 {
 	lock_msg();
 
+	if (is_hw(err))
+		++log_hardware_error_count;
+
 	if (stdlog) {
 		if (is_hw(err))
 			fprintf(stdlog, "msg:error_hardware: ");
@@ -470,6 +502,9 @@ void vlog_info(int err, const char* format, va_list ap, const char* post)
 {
 	lock_msg();
 
+	if (is_hw(err))
+		++log_hardware_error_count;
+
 	if (stdlog) {
 		if (is_hw(err))
 			fprintf(stdlog, "msg:error_hardware: "); /* we never expect an hardware error */
@@ -536,6 +571,9 @@ void log_expected(int err, const char* format, ...)
 
 	lock_msg();
 
+	if (is_hw(err))
+		++log_hardware_error_count;
+
 	if (stdlog) {
 		if (is_hw(err))
 			fprintf(stdlog, "msg:error_hardware: "); /* we never expect an hardware error */

cmdline/support.h

@@ -73,6 +73,11 @@ uint64_t random_u64(void);
  */
 int is_hw(int err);
 
+/**
+ * Return the number of hardware errors logged so far.
+ */
+unsigned log_hardware_errors(void);
+
 /**
  * Log errors with standard messages
  */

cmdline/sync.c

@@ -1767,7 +1767,7 @@ end:
 	*sync_durable = 1;
 
 	if (state->opt.kill_after_sync) {
-		log_fatal(EUSER, "WARNING! Killing due --test-kill-after-sync option.\n");
+		log_error(EUSER, "WARNING! Killing due --test-kill-after-sync option.\n");
 		exit(EXIT_SUCCESS);
 	}
 
@@ -2029,7 +2029,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t
 
 		/* if the file is too small */
 		if (out_size < used_parity_size) {
-			log_fatal(ESOFT, "WARNING! The %s parity has only %" PRIu64 " blocks instead of %" PRIu64 ".\n", lev_name(l), parityblocks, used_paritymax);
+			log_error(ESOFT, "WARNING! The %s parity has only %" PRIu64 " blocks instead of %" PRIu64 ".\n", lev_name(l), parityblocks, used_paritymax);
 		}
 
 		/* keep the smallest physical parity extent */
@@ -2152,7 +2152,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t
 		}
 
 		if (state->opt.kill_after_resize) {
-			log_fatal(EUSER, "WARNING! Killing due --test-kill-after-resize option.\n");
+			log_error(EUSER, "WARNING! Killing due --test-kill-after-resize option.\n");
 			exit(EXIT_SUCCESS);
 		}
 
@@ -2183,7 +2183,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t
 			if (state->need_write)
 				state_write(state);
 		} else {
-			log_fatal(EUSER, "WARNING! Skipped state write for --test-skip-content-write option.\n");
+			log_error(EUSER, "WARNING! Skipped state write for --test-skip-content-write option.\n");
 		}
 
 		/* make the scanned state available for recovery before changing parity */
@@ -2193,7 +2193,7 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t
 			/* continue, as we are already exiting */
 			/* LCOV_EXCL_STOP */
 		} else if (state->opt.kill_before_sync) {
-			log_fatal(EUSER, "WARNING! Killing due --test-kill-before-sync option.\n");
+			log_error(EUSER, "WARNING! Killing due --test-kill-before-sync option.\n");
 			exit(EXIT_SUCCESS);
 		} else if (blockstart < blockmax) {
 			ret = state_sync_process(state, parity_handle, blockstart, blockmax, &sync_durable);
@@ -2272,13 +2272,18 @@ int state_sync(struct snapraid_state* state, block_off_t blockstart, block_off_t
 		if (sync_durable) {
 			state_write(state);
 		} else {
-			log_fatal(EUSER, "WARNING! Skipping saving the content file because the sync was not durable.\n");
+			log_error(EUSER, "WARNING! Skipping saving the content file because the sync was not durable.\n");
 		}
 	}
 
 	if (process_error != 0)
 		return -1;
 
+	if (log_hardware_errors() != 0) {
+		msg_status("Everything done, but hardware errors were encountered in the process\n");
+		return -1;
+	}
+
 	msg_status("Everything OK\n");
 
 	return 0;

cmdline/touch.c

@@ -8,10 +8,11 @@
 #include "state.h"
 #include "handle.h"
 
-void state_touch(struct snapraid_state* state)
+int state_touch(struct snapraid_state* state)
 {
 	tommy_node* i;
 	unsigned counter = 0;
+	int error = 0;
 
 	msg_progress("Setting sub-second timestamps...\n");
 
@@ -80,6 +81,7 @@ void state_touch(struct snapraid_state* state)
 								f = open(path, flags);
 								if (windows_set_file_attributes(path, attributes) != 0) {
 									log_error(errno, "Error restoring read-only attribute for '%s'. %s.\n", path, strerror(errno));
+									++error;
 								}
 							}
 						}
@@ -90,6 +92,7 @@ void state_touch(struct snapraid_state* state)
 				if (f == -1) {
 					/* LCOV_EXCL_START */
 					log_error(errno, "Error opening file '%s'. %s.\n", path, strerror(errno));
+					++error;
 					continue;
 					/* LCOV_EXCL_STOP */
 				}
@@ -103,6 +106,7 @@ void state_touch(struct snapraid_state* state)
 					/* LCOV_EXCL_START */
 					close(f);
 					log_error(errno, "Error accessing file '%s'. %s.\n", path, strerror(errno));
+					++error;
 					continue;
 					/* LCOV_EXCL_STOP */
 				}
@@ -113,6 +117,7 @@ void state_touch(struct snapraid_state* state)
 					/* LCOV_EXCL_START */
 					close(f);
 					log_error(errno, "Error timing file '%s'. %s.\n", path, strerror(errno));
+					++error;
 					continue;
 					/* LCOV_EXCL_STOP */
 				}
@@ -127,6 +132,7 @@ void state_touch(struct snapraid_state* state)
 					/* LCOV_EXCL_START */
 					close(f);
 					log_error(errno, "Error accessing file '%s'. %s.\n", path, strerror(errno));
+					++error;
 					continue;
 					/* LCOV_EXCL_STOP */
 				}
@@ -136,6 +142,7 @@ void state_touch(struct snapraid_state* state)
 				if (ret != 0) {
 					/* LCOV_EXCL_START */
 					log_error(errno, "Error closing file '%s'. %s.\n", path, strerror(errno));
+					++error;
 					continue;
 					/* LCOV_EXCL_STOP */
 				}
@@ -161,5 +168,9 @@ void state_touch(struct snapraid_state* state)
 
 	msg_status("\n");
 	msg_status("%8u touched files\n", counter);
+
+	if (error != 0)
+		return -1;
+	return 0;
 }
 

cmdline/unixapp.c

@@ -4124,7 +4124,7 @@ int devquery(tommy_list* high, tommy_list* low)
 	/* sysfs interface is required */
 	if (stat("/sys/dev/block", &st) != 0) {
 		/* LCOV_EXCL_START */
-		log_fatal(EEXTERNAL, "Missing interface /sys/dev/block.\n");
+		log_error(EEXTERNAL, "Missing interface /sys/dev/block.\n");
 		return -1;
 		/* LCOV_EXCL_STOP */
 	}