Reviewed-by: Kostiantyn Kostiuk <[email protected]> Best Regards, Kostiantyn Kostiuk.
On Sat, Nov 1, 2025 at 12:42 PM Michael Tokarev <[email protected]> wrote: > The code checks existance of a command (halt/poweroff/reboot) by using > stat(2) and immediately checking for S_ISLNK() on the returned stat > struct. This check will never be true, because stat(2) always follows > symbolic links and hence will either return ENOENT (in case of dangling > symlink) or the properties for the final target file. It is lstat(2) > which might return information about the symlink itself. However, even > there, we want to check the final file properties, not the first symlink. > > This check - S_ISLNK - is harmful but useless in this case. However, it > is confusing and it helps the wrong usage of stat(2) to spread, so it is > better to remove it. > > Additionally, the code would better to check for the executable bits > of the final file, not check if it's a regular file - it's sort of > dubious to have anything but regular files in /sbin/. > > But a POSIX system provides another command which suits the purpose > perfectly: it is access(2). And it is so simple that it's not > necessary to create a separate function when usin it. > > Replace stat(2) with access(X_OK) to check for file existance in > qga/commands-posix.c > > Fixes: c5b4afd4d56e "qga: Support guest shutdown of BusyBox-based systems" > Signed-off-by: Michael Tokarev <[email protected]> > --- > qga/commands-posix.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index c7059857e4..0f4d6d96cc 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c > @@ -213,12 +213,6 @@ out: > return retcode; > } > > -static bool file_exists(const char *path) > -{ > - struct stat st; > - return stat(path, &st) == 0 && (S_ISREG(st.st_mode) || > S_ISLNK(st.st_mode)); > -} > - > #define POWEROFF_CMD_PATH "/sbin/poweroff" > #define HALT_CMD_PATH "/sbin/halt" > #define REBOOT_CMD_PATH "/sbin/reboot" > @@ -245,17 +239,17 @@ void qmp_guest_shutdown(const char *mode, Error > **errp) > > slog("guest-shutdown called, mode: %s", mode); > if (!mode || strcmp(mode, "powerdown") == 0) { > - if (file_exists(POWEROFF_CMD_PATH)) { > + if (access(POWEROFF_CMD_PATH, X_OK)) { > shutdown_cmd = POWEROFF_CMD_PATH; > } > shutdown_flag = powerdown_flag; > } else if (strcmp(mode, "halt") == 0) { > - if (file_exists(HALT_CMD_PATH)) { > + if (access(HALT_CMD_PATH, X_OK)) { > shutdown_cmd = HALT_CMD_PATH; > } > shutdown_flag = halt_flag; > } else if (strcmp(mode, "reboot") == 0) { > - if (file_exists(REBOOT_CMD_PATH)) { > + if (access(REBOOT_CMD_PATH, X_OK)) { > shutdown_cmd = REBOOT_CMD_PATH; > } > shutdown_flag = reboot_flag; > -- > 2.47.3 > >
