On Mon, Jun 07, 2021 at 02:10:48PM +0800, Luke Yue wrote:
Signed-off-by: Luke Yue <lukedyue@xxxxxxxxx>
---
src/util/virfile.c | 48 +++-------------------------------------------
1 file changed, 3 insertions(+), 45 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 7fe357ab16..14b45f4e1b 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -1662,55 +1662,13 @@ virFileIsLink(const char *linkpath)
char *
virFindFileInPath(const char *file)
{
- const char *origpath = NULL;
- g_auto(GStrv) paths = NULL;
- char **pathiter;
-
+ g_autofree char *path = NULL;
if (file == NULL)
return NULL;
- /* if we are passed an absolute path (starting with /), return a
- * copy of that path, after validating that it is executable
- */
- if (g_path_is_absolute(file)) {
- if (!virFileIsExecutable(file))
- return NULL;
-
- return g_strdup(file);
- }
-
- /* If we are passed an anchored path (containing a /), then there
- * is no path search - it must exist in the current directory
- */
- if (strchr(file, '/')) {
- char *abspath = NULL;
-
- if (!virFileIsExecutable(file))
- return NULL;
-
- ignore_value(abspath = g_canonicalize_filename(file, NULL));
- return abspath;
Oh, I see my fixup does not need to be made here since it is removed anyway. It is not completely pointless to have the code clean even between commits, although you already know and do that, but just to point out it can sometimes happen that the second patch needs to be reverted in which case the code would end up not as clean as possible, but that is very rarely the case and if that happens it is usually with more complex code I imagine. So no big deal here.
- }
+ path = g_find_program_in_path(file);
- /* copy PATH env so we can tweak it */
- origpath = getenv("PATH");
- if (!origpath)
- origpath = "/bin:/usr/bin";
-
- /* for each path segment, append the file to search for and test for
- * it. return it if found.
- */
-
- if (!(paths = g_strsplit(origpath, ":", 0)))
- return NULL;
-
- for (pathiter = paths; *pathiter; pathiter++) {
- g_autofree char *fullpath = g_build_filename(*pathiter, file, NULL);
- if (virFileIsExecutable(fullpath))
- return g_steal_pointer(&fullpath);
- }
-
- return NULL;
+ return g_canonicalize_filename(path, NULL);
Since you did not go with the glibcompat approach, it would be nice to add a comment here describing why this is done as we might be baffled by this in a couple of years when glib fix for the g_find_program_in_path() reaches all distros. I'll mention it here before pushing. Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
} -- 2.31.1
Attachment:
signature.asc
Description: PGP signature
