[PATCH 3/3] virfile: Use g_canonicalize_file() to simplify virFileAbsPath()

Luke Yue posted 3 patches 4 years, 8 months ago
[PATCH 3/3] virfile: Use g_canonicalize_file() to simplify virFileAbsPath()
Posted by Luke Yue 4 years, 8 months ago
Though the comment says that the function may return -1 on error, but it
seems that now it will never return -1 now. So just use g_canonicalize_file()
to simplify the implementation.

Signed-off-by: Luke Yue <lukedyue@gmail.com>
---
 src/util/virfile.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 0d1c2ba518..bfff471194 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3126,13 +3126,7 @@ virFileOpenTty(int *ttyprimary G_GNUC_UNUSED,
 int
 virFileAbsPath(const char *path, char **abspath)
 {
-    if (g_path_is_absolute(path)) {
-        *abspath = g_strdup(path);
-    } else {
-        g_autofree char *buf = g_get_current_dir();
-
-        *abspath = g_build_filename(buf, path, NULL);
-    }
+    *abspath = g_canonicalize_filename(path, NULL);
 
     return 0;
 }
-- 
2.31.1

Re: [PATCH 3/3] virfile: Use g_canonicalize_file() to simplify virFileAbsPath()
Posted by Martin Kletzander 4 years, 8 months ago
On Mon, May 31, 2021 at 09:48:24AM +0800, Luke Yue wrote:
>Though the comment says that the function may return -1 on error, but it
>seems that now it will never return -1 now. So just use g_canonicalize_file()
>to simplify the implementation.
>

Yeah, that is a leftover from before we started using glib and
abort()'ing on OOM.  It would be nice if that leftover got cleaned up as
well.  However, looking at it, we can remove the function altogether and
just use the glib counterpart.  That'd be even more of a clean up ;)

>Signed-off-by: Luke Yue <lukedyue@gmail.com>
>---
> src/util/virfile.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
>diff --git a/src/util/virfile.c b/src/util/virfile.c
>index 0d1c2ba518..bfff471194 100644
>--- a/src/util/virfile.c
>+++ b/src/util/virfile.c
>@@ -3126,13 +3126,7 @@ virFileOpenTty(int *ttyprimary G_GNUC_UNUSED,
> int
> virFileAbsPath(const char *path, char **abspath)
> {
>-    if (g_path_is_absolute(path)) {
>-        *abspath = g_strdup(path);
>-    } else {
>-        g_autofree char *buf = g_get_current_dir();
>-
>-        *abspath = g_build_filename(buf, path, NULL);
>-    }
>+    *abspath = g_canonicalize_filename(path, NULL);
>
>     return 0;
> }
>-- 
>2.31.1
>
Re: [PATCH 3/3] virfile: Use g_canonicalize_file() to simplify virFileAbsPath()
Posted by Luke Yue 4 years, 8 months ago
On Fri, 2021-06-04 at 12:28 +0200, Martin Kletzander wrote:
> On Mon, May 31, 2021 at 09:48:24AM +0800, Luke Yue wrote:
> > Though the comment says that the function may return -1 on error,
> > but it
> > seems that now it will never return -1 now. So just use
> > g_canonicalize_file()
> > to simplify the implementation.
> > 
> 
> Yeah, that is a leftover from before we started using glib and
> abort()'ing on OOM.  It would be nice if that leftover got cleaned up
> as
> well.  However, looking at it, we can remove the function altogether
> and
> just use the glib counterpart.  That'd be even more of a clean up ;)

Thanks for the review!

I will try to replace all the funciton calls with g_canonicalize_file()
and remove the function in a new patch.

Luke