From nobody Fri Nov 14 17:02:56 2025 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1762002948779582.032065757482; Sat, 1 Nov 2025 06:15:48 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1vFBS2-0007wl-Rw; Sat, 01 Nov 2025 09:15:30 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vFBRn-0007r0-Aj; Sat, 01 Nov 2025 09:15:11 -0400 Received: from isrv.corpit.ru ([212.248.84.144]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1vFBRc-0001w3-Kz; Sat, 01 Nov 2025 09:15:07 -0400 Received: from tsrv.corpit.ru (tsrv.tls.msk.ru [192.168.177.2]) by isrv.corpit.ru (Postfix) with ESMTP id 791381652E6; Sat, 01 Nov 2025 16:14:45 +0300 (MSK) Received: from think4mjt.tls.msk.ru (mjtthink.wg.tls.msk.ru [192.168.177.146]) by tsrv.corpit.ru (Postfix) with ESMTP id 402FE309F2C; Sat, 01 Nov 2025 16:14:57 +0300 (MSK) From: Michael Tokarev To: qemu-devel@nongnu.org Cc: Michael Tokarev , qemu-trivial@nongnu.org, Rodrigo Dias Correa , Kostiantyn Kostiuk Subject: [PATCH trivial v3] qga: use access(2) to check for command existance instead of questionable stat(2) Date: Sat, 1 Nov 2025 16:14:55 +0300 Message-ID: <20251101131456.792868-1-mjt@tls.msk.ru> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=212.248.84.144; envelope-from=mjt@tls.msk.ru; helo=isrv.corpit.ru X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZM-MESSAGEID: 1762002953787154100 Content-Type: text/plain; charset="utf-8" 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 Reviewed-by: Kostiantyn Kostiuk Reviewed-by: Philippe Mathieu-Daud=C3=A9 Reviewed-by: Rodrigo Dias Correa --- v3: and actually don't forget to commit the changes. I'm sorry for the noize. v2: fix reverse logic of the access() tests. I should write code more often :) 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..7785150fe4 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -213,12 +213,6 @@ out: return retcode; } =20 -static bool file_exists(const char *path) -{ - struct stat st; - return stat(path, &st) =3D=3D 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 **err= p) =20 slog("guest-shutdown called, mode: %s", mode); if (!mode || strcmp(mode, "powerdown") =3D=3D 0) { - if (file_exists(POWEROFF_CMD_PATH)) { + if (access(POWEROFF_CMD_PATH, X_OK) =3D=3D 0) { shutdown_cmd =3D POWEROFF_CMD_PATH; } shutdown_flag =3D powerdown_flag; } else if (strcmp(mode, "halt") =3D=3D 0) { - if (file_exists(HALT_CMD_PATH)) { + if (access(HALT_CMD_PATH, X_OK) =3D=3D 0) { shutdown_cmd =3D HALT_CMD_PATH; } shutdown_flag =3D halt_flag; } else if (strcmp(mode, "reboot") =3D=3D 0) { - if (file_exists(REBOOT_CMD_PATH)) { + if (access(REBOOT_CMD_PATH, X_OK) =3D=3D 0) { shutdown_cmd =3D REBOOT_CMD_PATH; } shutdown_flag =3D reboot_flag; --=20 2.47.3