The g_path_is_absolute() considers more situations
than just a simply "path[0] == '/'".

Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12

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

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 93fac200cc..3311eaff3d 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3153,7 +3153,7 @@ virFileOpenTty(int *ttyprimary G_GNUC_UNUSED,
 int
 virFileAbsPath(const char *path, char **abspath)
 {
-    if (path[0] == '/') {
+    if (g_path_is_absolute(path)) {
         *abspath = g_strdup(path);
     } else {
         g_autofree char *buf = g_get_current_dir();
-- 
2.31.1

On 4/12/21 4:04 PM, Luke Yue wrote:
> The g_path_is_absolute() considers more situations
> than just a simply "path[0] == '/'".
> 
> Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
> 
> Signed-off-by: Luke Yue <lukedyue@gmail.com>
> ---
>   src/util/virfile.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 93fac200cc..3311eaff3d 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3153,7 +3153,7 @@ virFileOpenTty(int *ttyprimary G_GNUC_UNUSED,
>   int
>   virFileAbsPath(const char *path, char **abspath)
>   {
> -    if (path[0] == '/') {
> +    if (g_path_is_absolute(path)) {
>           *abspath = g_strdup(path);
>       } else {
>           g_autofree char *buf = g_get_current_dir();
> 

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

and congratulations on your first libvirt contribution!

Although, there are more places like this. Quick git grep -n 
"\[0\].*'/'"  shows something. Can you post patch for them too? Maybe 
not every occurrence needs the same treatment, I know.

Michal

Thanks for the review!

I will look into them in my spare time, and I have a small question.
Do I need to create a series of little patches? Cause the files that
need the treatment belong to different parts of the libvirt.
Or just squash them into one commit because the changes are not that large?

Zelong

Michal Privoznik <mprivozn@redhat.com> 于2021年4月13日周二 下午7:11写道：

>
> On 4/12/21 4:04 PM, Luke Yue wrote:
> > The g_path_is_absolute() considers more situations
> > than just a simply "path[0] == '/'".
> >
> > Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
> >
> > Signed-off-by: Luke Yue <lukedyue@gmail.com>
> > ---
> >   src/util/virfile.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/util/virfile.c b/src/util/virfile.c
> > index 93fac200cc..3311eaff3d 100644
> > --- a/src/util/virfile.c
> > +++ b/src/util/virfile.c
> > @@ -3153,7 +3153,7 @@ virFileOpenTty(int *ttyprimary G_GNUC_UNUSED,
> >   int
> >   virFileAbsPath(const char *path, char **abspath)
> >   {
> > -    if (path[0] == '/') {
> > +    if (g_path_is_absolute(path)) {
> >           *abspath = g_strdup(path);
> >       } else {
> >           g_autofree char *buf = g_get_current_dir();
> >
>
> Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
>
> and congratulations on your first libvirt contribution!
>
> Although, there are more places like this. Quick git grep -n
> "\[0\].*'/'"  shows something. Can you post patch for them too? Maybe
> not every occurrence needs the same treatment, I know.
>
> Michal
>

On 4/13/21 3:38 PM, Luke Yue wrote:
> Thanks for the review!
> 
> I will look into them in my spare time, and I have a small question.
> Do I need to create a series of little patches? Cause the files that
> need the treatment belong to different parts of the libvirt.
> Or just squash them into one commit because the changes are not that large?


One patch is okay. It's going to be one semantic change and thus it is 
okay if it's contained in a single patch.

Michal

OK, I got it, thanks!

Zelong

Michal Privoznik <mprivozn@redhat.com> 于2021年4月13日周二 下午9:40写道：
>
> On 4/13/21 3:38 PM, Luke Yue wrote:
> > Thanks for the review!
> >
> > I will look into them in my spare time, and I have a small question.
> > Do I need to create a series of little patches? Cause the files that
> > need the treatment belong to different parts of the libvirt.
> > Or just squash them into one commit because the changes are not that large?
>
>
> One patch is okay. It's going to be one semantic change and thus it is
> okay if it's contained in a single patch.
>
> Michal
>