From c1bcd6a24f6ec1f7410b1a0e2e99aa4f4e90deda Mon Sep 17 00:00:00 2001 From: Andrea Mazzoleni Date: Mon, 14 Sep 2026 15:40:54 +0200 Subject: [PATCH] Migrate Windows smartctl execution from _wpopen to os_popen and direct process spawning --- cmdline/app.h | 9 +- cmdline/elem.h | 12 +- cmdline/mingwapp.c | 366 ++++++++++++++++++++++++++++++++++----------- cmdline/state.c | 4 +- cmdline/unixapp.c | 16 +- os/mingw.c | 81 +++++++++- os/os.h | 73 +++++---- os/unix.c | 6 +- 8 files changed, 433 insertions(+), 134 deletions(-) diff --git a/cmdline/app.h b/cmdline/app.h index 3e664fa3..a3046779 100644 --- a/cmdline/app.h +++ b/cmdline/app.h @@ -335,6 +335,11 @@ extern int exit_sync_needed; */ #define SMART_MAX 128 +/** + * Max smartctl custom option length. + */ +#define SMARTCTL_MAX 512 + /** * Smart ignore structure. */ @@ -379,8 +384,8 @@ struct devinfo_struct { uint64_t device; /**< Device ID. */ char name[PATH_MAX]; /**< Name of the disk combined with the split index if any */ char mount[PATH_MAX]; /**< Mount point or other contained directory. */ - char smartctl[SMART_MAX]; /**< Custom option for smartctl. Empty means auto. */ - char smartctl_info[SMART_MAX]; /* Info options for smartctl. Empty means -a. */ + char smartctl[SMARTCTL_MAX]; /**< Custom option for smartctl. Empty means auto. */ + char smartctl_info[SMARTCTL_MAX]; /* Info options for smartctl. Empty means -a. */ struct smartignore_struct smartignore[SMART_IGNORE_MAX]; /**< Attributes to ignore */ char file[PATH_MAX]; /**< File device. */ #ifdef _WIN32 diff --git a/cmdline/elem.h b/cmdline/elem.h index 126a7f09..f633c36d 100644 --- a/cmdline/elem.h +++ b/cmdline/elem.h @@ -403,8 +403,8 @@ struct snapraid_extent { struct snapraid_extra { char name[PATH_MAX]; /**< Name of the disk. */ char dir[PATH_MAX]; /**< Mount point of the disk. It always terminates with /. */ - char smartctl[SMART_MAX]; /**< Custom option for smartctl. Empty means auto. */ - char smartctl_info[SMART_MAX]; /* Info options for smartctl. Empty means -a. */ + char smartctl[SMARTCTL_MAX]; /**< Custom option for smartctl. Empty means auto. */ + char smartctl_info[SMARTCTL_MAX]; /* Info options for smartctl. Empty means -a. */ struct smartignore_struct smartignore[SMART_IGNORE_MAX]; /**< Smart attributes to ignore for this device. */ char uuid[UUID_MAX]; /**< UUID of the disk. They are probed during the config reading. */ char fstype[FSINFO_MAX]; /**< Filesystem type */ @@ -442,8 +442,8 @@ struct snapraid_disk { */ struct fssnapshot_struct fss; - char smartctl[SMART_MAX]; /**< Custom option for smartctl. Empty means auto. */ - char smartctl_info[SMART_MAX]; /* Info options for smartctl. Empty means -a. */ + char smartctl[SMARTCTL_MAX]; /**< Custom option for smartctl. Empty means auto. */ + char smartctl_info[SMARTCTL_MAX]; /* Info options for smartctl. Empty means -a. */ struct smartignore_struct smartignore[SMART_IGNORE_MAX]; /**< Smart attributes to ignore for this device. */ char uuid[UUID_MAX]; /**< UUID of the disk. They are probed during the config reading. */ char fstype[FSINFO_MAX]; /**< Filesystem type */ @@ -585,8 +585,8 @@ struct snapraid_split { struct snapraid_parity { struct snapraid_split split_map[SPLIT_MAX]; /**< Parity splits. */ unsigned split_mac; /**< Number of parity splits. */ - char smartctl[SMART_MAX]; /**< Custom option for smartctl. Empty means auto. */ - char smartctl_info[SMART_MAX]; /* Info options for smartctl. Empty means -a. */ + char smartctl[SMARTCTL_MAX]; /**< Custom option for smartctl. Empty means auto. */ + char smartctl_info[SMARTCTL_MAX]; /* Info options for smartctl. Empty means -a. */ struct smartignore_struct smartignore[SMART_IGNORE_MAX]; /**< Smart attributes to ignore for this device. */ block_off_t total_blocks; /**< Number of total blocks. */ block_off_t free_blocks; /**< Number of free blocks at the last sync. */ diff --git a/cmdline/mingwapp.c b/cmdline/mingwapp.c index 0d172387..5b88dc13 100644 --- a/cmdline/mingwapp.c +++ b/cmdline/mingwapp.c @@ -248,7 +248,11 @@ static int base64_encode(const unsigned char* in, size_t in_len, char* out, size * * Runs a PowerShell one-liner via os_spawn() and captures the first line * of stdout into `out` as UTF-8. - * `out` may be NULL when output is not needed. + * `out` may be 0 when output is not needed. + * + * Because os_spawn() captures stdout as a raw binary file descriptor without + * CRLF translation, callers must handle Windows \r\n explicitly. Here, \r\n + * is stripped using strpbrk() and strtrim(). * * The command is executed using -EncodedCommand (Base64 UTF-16LE) to safely * handle any shell metacharacters or quotes in paths and volume names. @@ -1117,61 +1121,145 @@ static int devstat(uint64_t device, const char* name, const char* wfile, uint64_ return 0; } +#define ARGS_MAX 64 + +/** + * Build the absolute path to smartctl.exe located in the application directory. + */ +static int smartctl_executable(char* path, size_t size) +{ + char conv_buf[CONV_MAX]; + const wchar_t* dir = windows_exedir(); + int ret; + + if (!dir || dir[0] == 0) { + errno = ENOENT; + return -1; + } + + ret = snprintf(path, size, "%ssmartctl.exe", u16tou8(conv_buf, dir)); + if (ret < 0 || (size_t)ret >= size) { + errno = ENAMETOOLONG; + return -1; + } + + return 0; +} + /** * Get SMART attributes. */ static int devsmart(uint64_t device, const char* name, const char* smartctl, const char* smartctl_info, struct smart_attr* smart, uint64_t* info, char* serial, char* family, char* model, char* inter) { - char conv_buf[CONV_MAX]; - WCHAR cmd[PATH_MAX + SMART_MAX]; + char smartctl_path[CONV_MAX]; + char info_buf[SMARTCTL_MAX]; + char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; + const char* argv[ARGS_MAX]; char file[128]; - FILE* f; + unsigned argc; + OS_FILE* f; int ret; int count; + if (smartctl_executable(smartctl_path, sizeof(smartctl_path)) != 0) { + /* LCOV_EXCL_START */ + log_fatal(errno, "Failed to build smartctl executable path.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + snprintf(file, sizeof(file), "/dev/pd%" PRIu64, device); const char* info_opts = smartctl_info[0] ? smartctl_info : "-a"; - /* if there is a custom smartctl command */ - if (smartctl[0]) { - char option[SMART_MAX]; - snprintf(option, sizeof(option), smartctl, file); - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" %s %s", windows_exedir(), info_opts, option); - } else { - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" %s %s", windows_exedir(), info_opts, file); - } - count = 0; retry: - log_tag("smartctl:%s:%s:run: %s\n", file, name, u16tou8(conv_buf, cmd)); + argc = 0; + argv[argc++] = smartctl_path; - f = _wpopen(cmd, L"rt"); + pathcpy(info_buf, sizeof(info_buf), info_opts); + argc += argsplit(argv + argc, ARGS_MAX - argc, info_buf); + if (argc >= ARGS_MAX) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + + if (count != 0) { + if (argc >= ARGS_MAX - 3) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = "-d"; + argv[argc++] = "sat"; + } + + if (smartctl[0] && count == 0) { + pathprint(extra_args, sizeof(extra_args), smartctl, file); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); + if (argc >= ARGS_MAX) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + log_tag("smartctl:%s:%s:run: %s [info: %s] %s\n", file, name, smartctl_path, info_opts, extra_args); + } else { + if (argc >= ARGS_MAX - 1) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = file; + if (count != 0) + log_tag("smartctl:%s:%s:run: %s [info: %s] -d sat %s\n", file, name, smartctl_path, info_opts, file); + else + log_tag("smartctl:%s:%s:run: %s [info: %s] %s\n", file, name, smartctl_path, info_opts, file); + } + argv[argc++] = 0; + + f = os_popen(argv); if (!f) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from popen).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:spawn\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] %s'.\n", smartctl_path, info_opts, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] -d sat %s'.\n", smartctl_path, info_opts, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] %s'.\n", smartctl_path, info_opts, file); return -1; /* LCOV_EXCL_STOP */ } if (smartctl_attribute(f, file, name, smart, info, serial, family, model, inter) != 0) { /* LCOV_EXCL_START */ - pclose(f); - log_tag("device:%s:%s:shell\n", file, name); + os_pclose(f); + log_tag("device:%s:%s:spawn\n", file, name); return -1; /* LCOV_EXCL_STOP */ } - ret = pclose(f); + ret = os_pclose(f); log_tag("smartctl:%s:%s:ret: %x\n", file, name, ret); - if (ret == -1) { + if (!WIFEXITED(ret)) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from pclose).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:abort\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] %s' (not exited).\n", smartctl_path, info_opts, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] -d sat %s' (not exited).\n", smartctl_path, info_opts, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s [info: %s] %s' (not exited).\n", smartctl_path, info_opts, file); return -1; /* LCOV_EXCL_STOP */ } @@ -1189,21 +1277,18 @@ retry: * * Note that getting error 4 is instead very common, even with full info gathering. */ - if ((ret == 0 || ret == 2) + if ((WEXITSTATUS(ret) == 0 || WEXITSTATUS(ret) == 2) && smart[SMART_POWER_ON_HOURS].raw == SMART_UNASSIGNED && info[INFO_SIZE] == SMART_UNASSIGNED && info[INFO_ROTATION_RATE] == SMART_UNASSIGNED ) { - /* retry using the "sat" type */ - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" %s -d sat %s", windows_exedir(), info_opts, file); - ++count; goto retry; } } /* store the smartctl return value */ - smart[SMART_FLAGS].raw = ret; + smart[SMART_FLAGS].raw = WEXITSTATUS(ret); return 0; } @@ -1495,56 +1580,117 @@ static int devpower(uint64_t device, const char* name, const char* wfile) */ static int devprobe(uint64_t device, const char* name, const char* smartctl, const char* smartctl_info, int* power, struct smart_attr* smart, uint64_t* info, char* serial, char* family, char* model, char* interf) { - char conv_buf[CONV_MAX]; - WCHAR cmd[PATH_MAX + SMART_MAX]; + char smartctl_path[CONV_MAX]; + char info_buf[SMARTCTL_MAX]; + char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; + const char* argv[ARGS_MAX]; char file[128]; - FILE* f; + unsigned argc; + OS_FILE* f; int ret; int count; + if (smartctl_executable(smartctl_path, sizeof(smartctl_path)) != 0) { + /* LCOV_EXCL_START */ + log_fatal(errno, "Failed to build smartctl executable path.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + snprintf(file, sizeof(file), "/dev/pd%" PRIu64, device); const char* info_opts = smartctl_info[0] ? smartctl_info : "-a"; - /* if there is a custom smartctl command */ - if (smartctl[0]) { - char option[SMART_MAX]; - snprintf(option, sizeof(option), smartctl, file); - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -n standby,3 %s %s", windows_exedir(), info_opts, option); - } else { - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -n standby,3 %s %s", windows_exedir(), info_opts, file); - } - count = 0; retry: - log_tag("smartctl:%s:%s:run: %s\n", file, name, u16tou8(conv_buf, cmd)); + argc = 0; + argv[argc++] = smartctl_path; + argv[argc++] = "-n"; + argv[argc++] = "standby,3"; - f = _wpopen(cmd, L"rt"); + pathcpy(info_buf, sizeof(info_buf), info_opts); + argc += argsplit(argv + argc, ARGS_MAX - argc, info_buf); + if (argc >= ARGS_MAX) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + + if (count != 0) { + if (argc >= ARGS_MAX - 3) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = "-d"; + argv[argc++] = "sat"; + } + + if (smartctl[0] && count == 0) { + pathprint(extra_args, sizeof(extra_args), smartctl, file); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); + if (argc >= ARGS_MAX) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + log_tag("smartctl:%s:%s:run: %s -n standby,3 [info: %s] %s\n", file, name, smartctl_path, info_opts, extra_args); + } else { + if (argc >= ARGS_MAX - 1) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = file; + if (count != 0) + log_tag("smartctl:%s:%s:run: %s -n standby,3 [info: %s] -d sat %s\n", file, name, smartctl_path, info_opts, file); + else + log_tag("smartctl:%s:%s:run: %s -n standby,3 [info: %s] %s\n", file, name, smartctl_path, info_opts, file); + } + argv[argc++] = 0; + + f = os_popen(argv); if (!f) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from popen).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:spawn\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s %s'.\n", smartctl_path, info_opts, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s -d sat %s'.\n", smartctl_path, info_opts, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s %s'.\n", smartctl_path, info_opts, file); return -1; /* LCOV_EXCL_STOP */ } if (smartctl_attribute(f, file, name, smart, info, serial, family, model, interf) != 0) { /* LCOV_EXCL_START */ - pclose(f); - log_tag("device:%s:%s:shell\n", file, name); + os_pclose(f); + log_tag("device:%s:%s:spawn\n", file, name); return -1; /* LCOV_EXCL_STOP */ } - ret = pclose(f); + ret = os_pclose(f); log_tag("smartctl:%s:%s:ret: %x\n", file, name, ret); - if (ret == -1) { + if (!WIFEXITED(ret)) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from pclose).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:abort\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s %s' (not exited).\n", smartctl_path, info_opts, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s -d sat %s' (not exited).\n", smartctl_path, info_opts, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s -n standby,3 %s %s' (not exited).\n", smartctl_path, info_opts, file); return -1; /* LCOV_EXCL_STOP */ } @@ -1559,16 +1705,13 @@ retry: * * In such conditions we retry using the "sat" type, that often allows to proceed. */ - if (ret == 2) { - /* retry using the "sat" type */ - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -n standby,3 %s -d sat %s", windows_exedir(), info_opts, file); - + if (WEXITSTATUS(ret) == 2) { ++count; goto retry; } } - if (ret == 3) { + if (WEXITSTATUS(ret) == 3) { log_tag("attr:%s:%s:power:standby\n", file, name); *power = POWER_STANDBY; } else { @@ -1577,7 +1720,7 @@ retry: /* store the smartctl return value */ if (smart) - smart[SMART_FLAGS].raw = ret; + smart[SMART_FLAGS].raw = WEXITSTATUS(ret); } return 0; @@ -1588,54 +1731,105 @@ retry: */ static int devdown(uint64_t device, const char* name, const char* smartctl) { - char conv_buf[CONV_MAX]; - WCHAR cmd[PATH_MAX + SMART_MAX]; + char smartctl_path[CONV_MAX]; + char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; + const char* argv[ARGS_MAX]; char file[128]; - FILE* f; + unsigned argc; + OS_FILE* f; int ret; int count; - snprintf(file, sizeof(file), "/dev/pd%" PRIu64, device); - - /* if there is a custom smartctl command */ - if (smartctl[0]) { - char option[SMART_MAX]; - snprintf(option, sizeof(option), smartctl, file); - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -s standby,now %s", windows_exedir(), option); - } else { - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -s standby,now %s", windows_exedir(), file); + if (smartctl_executable(smartctl_path, sizeof(smartctl_path)) != 0) { + /* LCOV_EXCL_START */ + log_fatal(errno, "Failed to build smartctl executable path.\n"); + return -1; + /* LCOV_EXCL_STOP */ } + snprintf(file, sizeof(file), "/dev/pd%" PRIu64, device); + count = 0; retry: - log_tag("smartctl:%s:%s:run: %s\n", file, name, u16tou8(conv_buf, cmd)); + argc = 0; + argv[argc++] = smartctl_path; + argv[argc++] = "-s"; + argv[argc++] = "standby,now"; - f = _wpopen(cmd, L"rt"); + if (count != 0) { + if (argc >= ARGS_MAX - 3) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = "-d"; + argv[argc++] = "sat"; + } + + if (smartctl[0] && count == 0) { + pathprint(extra_args, sizeof(extra_args), smartctl, file); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); + if (argc >= ARGS_MAX) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + log_tag("smartctl:%s:%s:run: %s -s standby,now %s\n", file, name, smartctl_path, extra_args); + } else { + if (argc >= ARGS_MAX - 1) { + /* LCOV_EXCL_START */ + log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); + return -1; + /* LCOV_EXCL_STOP */ + } + argv[argc++] = file; + if (count != 0) + log_tag("smartctl:%s:%s:run: %s -s standby,now -d sat %s\n", file, name, smartctl_path, file); + else + log_tag("smartctl:%s:%s:run: %s -s standby,now %s\n", file, name, smartctl_path, file); + } + argv[argc++] = 0; + + f = os_popen(argv); if (!f) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from popen).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:spawn\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s'.\n", smartctl_path, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now -d sat %s'.\n", smartctl_path, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s'.\n", smartctl_path, file); return -1; /* LCOV_EXCL_STOP */ } if (smartctl_flush(f, file, name) != 0) { /* LCOV_EXCL_START */ - pclose(f); - log_tag("device:%s:%s:shell\n", file, name); + os_pclose(f); + log_tag("device:%s:%s:spawn\n", file, name); return -1; /* LCOV_EXCL_STOP */ } - ret = pclose(f); + ret = os_pclose(f); log_tag("smartctl:%s:%s:ret: %x\n", file, name, ret); - if (ret == -1) { + if (!WIFEXITED(ret)) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:shell\n", file, name); - log_fatal(errno, "Failed to run '%s' (from pclose).\n", u16tou8(conv_buf, cmd)); + log_tag("device:%s:%s:abort\n", file, name); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s' (not exited).\n", smartctl_path, extra_args); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now -d sat %s' (not exited).\n", smartctl_path, file); + else + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s' (not exited).\n", smartctl_path, file); return -1; /* LCOV_EXCL_STOP */ } @@ -1650,19 +1844,21 @@ retry: * * In such conditions we retry using the "sat" type, that often allows to proceed. */ - if (ret == 2) { - /* retry using the "sat" type */ - snwprintf(cmd, sizeof(cmd) / sizeof(cmd[0]), L"\"%lssmartctl.exe\" -s standby,now -d sat %s", windows_exedir(), file); - + if (WEXITSTATUS(ret) == 2) { ++count; goto retry; } } - if (ret != 0) { + if (WEXITSTATUS(ret) != 0) { /* LCOV_EXCL_START */ - log_tag("device:%s:%s:exit:%d\n", file, name, ret); - log_fatal(errno, "Failed to run '%s' with return code %xh.\n", u16tou8(conv_buf, cmd), ret); + log_tag("device:%s:%s:exit:%d\n", file, name, WEXITSTATUS(ret)); + if (smartctl[0] && count == 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s' with return code %xh.\n", smartctl_path, extra_args, WEXITSTATUS(ret)); + else if (count != 0) + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now -d sat %s' with return code %xh.\n", smartctl_path, file, WEXITSTATUS(ret)); + else + log_fatal(EEXTERNAL, "Failed to run '%s -s standby,now %s' with return code %xh.\n", smartctl_path, file, WEXITSTATUS(ret)); return -1; /* LCOV_EXCL_STOP */ } diff --git a/cmdline/state.c b/cmdline/state.c index d9714552..45bf789e 100644 --- a/cmdline/state.c +++ b/cmdline/state.c @@ -1202,8 +1202,8 @@ void state_config(struct snapraid_state* state, const char* path, const char* co tommy_list_insert_tail(&state->extralist, &extra->node, extra); } else if (strcmp(tag, "smartctl") == 0) { char custom[PATH_MAX]; - char parsed_smartctl[SMART_MAX]; - char parsed_smartctl_info[SMART_MAX]; + char parsed_smartctl[SMARTCTL_MAX]; + char parsed_smartctl_info[SMARTCTL_MAX]; ret = sgettok(f, buffer, sizeof(buffer)); if (ret < 0) { diff --git a/cmdline/unixapp.c b/cmdline/unixapp.c index e4b86032..f3377ba3 100644 --- a/cmdline/unixapp.c +++ b/cmdline/unixapp.c @@ -3285,7 +3285,7 @@ static int devsmart(dev_t device, const char* name, const char* smartctl, const unsigned argc = 0; argv[argc++] = x; - char info_buf[SMART_MAX]; + char info_buf[SMARTCTL_MAX]; pathcpy(info_buf, sizeof(info_buf), info_opts); argc += argsplit(argv + argc, ARGS_MAX - argc, info_buf); if (argc >= ARGS_MAX) { @@ -3296,9 +3296,11 @@ static int devsmart(dev_t device, const char* name, const char* smartctl, const } char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; if (smartctl[0]) { pathprint(extra_args, sizeof(extra_args), smartctl, file); - argc += argsplit(argv + argc, ARGS_MAX - argc, extra_args); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); if (argc >= ARGS_MAX) { /* LCOV_EXCL_START */ log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); @@ -3454,7 +3456,7 @@ static int devprobe(dev_t device, const char* name, const char* smartctl, const argv[argc++] = "-n"; argv[argc++] = "standby,3"; - char info_buf[SMART_MAX]; + char info_buf[SMARTCTL_MAX]; pathcpy(info_buf, sizeof(info_buf), info_opts); argc += argsplit(argv + argc, ARGS_MAX - argc, info_buf); if (argc >= ARGS_MAX) { @@ -3465,9 +3467,11 @@ static int devprobe(dev_t device, const char* name, const char* smartctl, const } char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; if (smartctl[0]) { pathprint(extra_args, sizeof(extra_args), smartctl, file); - argc += argsplit(argv + argc, ARGS_MAX - argc, extra_args); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); if (argc >= ARGS_MAX) { /* LCOV_EXCL_START */ log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); @@ -3545,6 +3549,7 @@ static int devprobe(dev_t device, const char* name, const char* smartctl, const static int devdown(dev_t device, const char* name, const char* smartctl) { char extra_args[PATH_MAX]; + char extra_split[PATH_MAX]; char file[PATH_MAX]; OS_FILE* f; int ret; @@ -3573,7 +3578,8 @@ static int devdown(dev_t device, const char* name, const char* smartctl) if (smartctl[0]) { pathprint(extra_args, sizeof(extra_args), smartctl, file); - argc += argsplit(argv + argc, ARGS_MAX - argc, extra_args); + pathcpy(extra_split, sizeof(extra_split), extra_args); + argc += argsplit(argv + argc, ARGS_MAX - argc, extra_split); if (argc >= ARGS_MAX) { /* LCOV_EXCL_START */ log_fatal(EEXTERNAL, "Too many smartctl arguments.\n"); diff --git a/os/mingw.c b/os/mingw.c index dbfe720d..044cb4c2 100644 --- a/os/mingw.c +++ b/os/mingw.c @@ -4150,9 +4150,88 @@ int os_randomize(void* void_ptr, size_t size) return -1; } +OS_FILE* os_popen(const char** argv) +{ + int stdout_fd = -1; + int saved_errno; + int status; + pid_t pid; + FILE* fp; + OS_FILE* os_file; + + os_file = malloc(sizeof(OS_FILE)); + if (!os_file) + return 0; + + /* os_spawn() only reads argv and its strings, so dropping const here does not allow the callee to modify them. */ + pid = os_spawn((char**)argv, &stdout_fd, 0, 0, 0); + if (pid < 0) { + free(os_file); + return 0; + } + + /* os_spawn() exposes Windows pipes as binary CRT descriptors; restore the text semantics previously provided by _wpopen(..., L"rt"). */ + if (_setmode(stdout_fd, O_TEXT) == -1) + goto fail_child; + + fp = fdopen(stdout_fd, "r"); + if (!fp) + goto fail_child; + + os_file->fp = fp; + os_file->pid = pid; + + return os_file; + +fail_child: + saved_errno = errno; + close(stdout_fd); + /* Terminate and reap the child immediately so no process or handle is leaked on stream creation failure. */ + pid_kill(pid); + status = 0; + os_wait(pid, &status, 0); + os_dispose(pid); + free(os_file); + errno = saved_errno; + return 0; +} + char* os_fgets(char* s, int size, OS_FILE* stream) { - return fgets(s, size, stream); + return fgets(s, size, stream->fp); +} + +int os_pclose(OS_FILE* stream) +{ + FILE* fp = stream->fp; + pid_t pid = stream->pid; + int status = 0; + int saved_errno = 0; + int close_ret; + pid_t wait_ret; + + /* Keep the owned resources locally so the OS_FILE container can be released immediately. */ + free(stream); + + /* Closing the stream also closes the CRT descriptor and the underlying stdout pipe handle. */ + close_ret = fclose(fp); + if (close_ret != 0) + saved_errno = errno; + + /* Always reap the child even if closing the stream failed. */ + wait_ret = os_wait(pid, &status, 0); + if (wait_ret < 0 && saved_errno == 0) + saved_errno = errno; + + /* os_wait() does not release the Windows process HANDLE. */ + os_dispose(pid); + + if (close_ret != 0 || wait_ret < 0) { + errno = saved_errno != 0 ? saved_errno : EIO; + return -1; + } + + return status; } void os_clear(void) diff --git a/os/os.h b/os/os.h index dfce4ad6..4fcb9800 100644 --- a/os/os.h +++ b/os/os.h @@ -59,14 +59,10 @@ int os_signal_interrupt(void); * This structure encapsulates a standard buffered I/O stream tied to the * redirected standard output of a child process spawned via os_popen(). */ -#ifdef __MINGW32__ -#define OS_FILE FILE -#else typedef struct OS_FILE { FILE* fp; /**< Standard I/O buffered stream wrapper. */ - pid_t pid; /**< Process ID of the child process. */ + pid_t pid; /**< Process reference of the child process. PID on Unix, process HANDLE on Windows. */ } OS_FILE; -#endif /** * Spawns a child process and opens a buffered stream to read its stdout. @@ -74,15 +70,19 @@ typedef struct OS_FILE { * This function acts as a safer alternative to standard popen(), accepting an * explicit argument vector rather than evaluating a shell command string. * - * The child process is completely isolated in its own process group (via setpgid) - * with its stdin and stderr redirected to /dev/null. The underlying pipe is - * created with O_CLOEXEC set to prevent leakage across concurrent forks. + * Standard input and standard error are discarded. Standard output is connected + * to a pipe exposed through the returned buffered stream. Process creation and + * argument serialization are delegated to the platform os_spawn() implementation. + * + * The stream is opened in text mode on all platforms (setting O_TEXT on Windows), + * ensuring that CRLF (\r\n) line endings emitted by Windows processes are automatically + * translated to LF (\n). Callers reading with os_fgets() can assume \n across platforms. * * The caller is expected to call os_privileges_acquire() before this operation if permission is needed. * - * \param argv A NULL-terminated array of strings representing the argument vector. + * \param argv A 0-terminated array of strings representing the argument vector. * argv[0] must contain the absolute path to the verified executable. - * \return A pointer to an initialized OS_FILE structure on success, or NULL on failure. + * \return A pointer to an initialized OS_FILE structure on success, or 0 on failure. */ OS_FILE* os_popen(const char** argv); @@ -94,11 +94,15 @@ OS_FILE* os_popen(const char** argv); * or an end-of-file (EOF) condition is encountered. A terminating null * character is appended. * + * Because os_popen() configures the stream in text mode, lines are uniformly + * terminated with \n (or end at EOF) without trailing \r carriage returns, + * even on Windows. + * * \param s Pointer to an array of chars where the string read is stored. * \param size Maximum number of characters to be read (including the null character). * \param stream Pointer to the OS_FILE tracking context. * \return On success, returns the buffer pointer s. If EOF is reached or a read - * error occurs before any characters are read, returns NULL. + * error occurs before any characters are read, returns 0. */ char* os_fgets(char* s, int size, OS_FILE* stream); @@ -106,8 +110,8 @@ char* os_fgets(char* s, int size, OS_FILE* stream); * Closes an execution stream and reaps the associated child process. * * Closes the underlying buffered standard I/O stream, releases the allocated - * tracking context container, and blocks until the associated child process - * terminates to prevent the creation of zombie processes. + * tracking context container, and waits until the associated child process + * terminates to collect its exit status and release its process resources. * * \param stream Pointer to the active OS_FILE context to close and free. * \return The termination status of the child process on success, or -1 on failure. @@ -119,8 +123,14 @@ int os_pclose(OS_FILE* stream); * * The caller is expected to call os_privileges_acquire() before this operation if permission is needed. * + * Line Ending Semantics: + * When stdout_read_fd or stderr_read_fd are captured, the resulting file descriptors are opened in raw + * binary mode (O_BINARY on Windows) without CRLF-to-LF translation. Callers that read directly from + * these descriptors must explicitly handle \r\n line terminators on Windows (or switch mode via _setmode()). + * For automatic \r\n to \n translation, use os_popen() and os_fgets(). + * * Process Reference Slots (pid_slot): - * If pid_slot is not NULL, process creation and the resulting process reference are published through it + * If pid_slot is not 0, process creation and the resulting process reference are published through it * to enable safe, race-free process tracking and termination (os_term, os_kill) across concurrent threads. * If os_kill() was called on pid_slot, process creation is rejected immediately and this function * returns -1 with errno set to ECANCELED without spawning a child process. In contrast, os_term() on an @@ -128,10 +138,10 @@ int os_pclose(OS_FILE* stream); * The same pid_slot pointer should be passed to os_wait() to unpublish it when the process terminates. * * \param argv Array of command line arguments. - * \param stdout_read_fd Pointer to store file descriptor for stdout, or NULL to redirect to /dev/null. - * \param stderr_read_fd Pointer to store file descriptor for stderr, or NULL to redirect to /dev/null. - * \param run_as_user User to run script as (NULL for current user). - * \param pid_slot Optional pointer used to publish the process reference, or NULL if concurrent termination is not required. + * \param stdout_read_fd Pointer to store file descriptor for stdout, or 0 to redirect to /dev/null. + * \param stderr_read_fd Pointer to store file descriptor for stderr, or 0 to redirect to /dev/null. + * \param run_as_user User to run script as (0 for current user). + * \param pid_slot Optional pointer used to publish the process reference, or 0 if concurrent termination is not required. * \return Process reference of spawned process, or -1 on failure. */ pid_t os_spawn(char** argv, int* stdout_read_fd, int* stderr_read_fd, const char* run_as_user, pid_t* pid_slot); @@ -157,12 +167,12 @@ uint64_t os_slot_pid(const pid_t* pid_slot); /** * Wait for the child process to terminate. - * If pid_slot is not NULL, the process reference is unpublished before this function returns. + * If pid_slot is not 0, the process reference is unpublished before this function returns. * This function does not release the process reference returned by os_spawn(). * The caller must eventually call os_dispose(). * \param pid Process reference returned by os_spawn(). * \param status Pointer to store the exit status. - * \param pid_slot Optional pointer previously passed to os_spawn(), or NULL if the process was not published. + * \param pid_slot Optional pointer previously passed to os_spawn(), or 0 if the process was not published. * \return Child process reference on success, -1 on failure. */ pid_t os_wait(pid_t pid, int* status, pid_t* pid_slot); @@ -209,16 +219,17 @@ int os_kill(pid_t* pid_slot); /** * Fork and execute a verified executable, discarding all I/O. * - * Spawns @argv[0] in a new process with stdin, stdout and stderr all + * Spawns @argv[0] via os_spawn() in a new process with stdin, stdout and stderr all * redirected to /dev/null. Use this for fire-and-forget tasks where the - * child's output is not needed. + * child's output is not needed. Because all I/O streams are discarded, no stream + * or line-ending translation is performed. * * The child is placed in its own process group (setpgid) to isolate it * from signals sent to the daemon's process group. * * The caller is expected to call os_privileges_acquire() before this operation if permission is needed. * - * \param argv NULL-terminated argument vector. argv[0] must be the absolute path + * \param argv 0-terminated argument vector. argv[0] must be the absolute path * to the executable. * \return The child exit status on success, or -1 on failure. */ @@ -227,13 +238,13 @@ int os_spawn_and_wait(const char** argv); /** * Execute a system command with optional user context and input. * The caller is expected to call os_privileges_acquire() before this operation if permission is needed. - * If pid_slot is not NULL, the child process is published for concurrent termination while blocked. + * If pid_slot is not 0, the child process is published for concurrent termination while blocked. * If os_kill() was called on pid_slot, process creation is rejected immediately and this function * returns -1 with errno set to ECANCELED without executing the command. * \param command Command to execute. - * \param run_as_user User to run command as (NULL for current user). - * \param stdin_text Text to provide as stdin (NULL for no input). - * \param pid_slot Optional pointer to publish the process reference while active, or NULL. + * \param run_as_user User to run command as (0 for current user). + * \param stdin_text Text to provide as stdin (0 for no input). + * \param pid_slot Optional pointer to publish the process reference while active, or 0. * \return Exit status of command, or -1 on failure. */ int os_command(const char* command, const char* run_as_user, const char* stdin_text, pid_t* pid_slot); @@ -241,13 +252,13 @@ int os_command(const char* command, const char* run_as_user, const char* stdin_t /** * Execute a script file with specified user context. * The caller is expected to call os_privileges_acquire() before this operation if permission is needed. - * If pid_slot is not NULL, the child process is published for concurrent termination while blocked. + * If pid_slot is not 0, the child process is published for concurrent termination while blocked. * If os_kill() was called on pid_slot, process creation is rejected immediately and this function * returns -1 with errno set to ECANCELED without executing the script. * \param argv Array of command line arguments. - * \param envp Environment variables (NULL-terminated list of strings). - * \param run_as_user User to run script as (NULL for current user). - * \param pid_slot Optional pointer to publish the process reference while active, or NULL. + * \param envp Environment variables (0-terminated list of strings). + * \param run_as_user User to run script as (0 for current user). + * \param pid_slot Optional pointer to publish the process reference while active, or 0. * \return Exit status of script, or -1 on failure. */ int os_script(char** argv, char** envp, const char* run_as_user, pid_t* pid_slot); diff --git a/os/unix.c b/os/unix.c index c0c74a03..2fdf0007 100644 --- a/os/unix.c +++ b/os/unix.c @@ -1524,11 +1524,13 @@ int os_command(const char* command, const char* run_as_user, const char* stdin_t /** * os_spawn() - Fork and execute a verified executable, capturing stdout and/or stderr. * - * Spawns @argv[0] in a new process. If @stdout_read_fd is not NULL, stdout is connected - * to a pipe whose read end is returned in @stdout_read_fd. If @stderr_read_fd is not NULL, + * Spawns @argv[0] in a new process. If @stdout_read_fd is not 0, stdout is connected + * to a pipe whose read end is returned in @stdout_read_fd. If @stderr_read_fd is not 0, * stderr is connected to a pipe whose read end is returned in @stderr_read_fd. * Otherwise, they are redirected to /dev/null. stdin is always redirected to /dev/null. * + * The captured descriptors are raw binary file descriptors without CRLF translation. + * * The child is placed in its own process group (setpgid) to isolate it from signals * sent to the daemon's process group. *