[PATCH resend] virfile: Replace AbsPath judgement method with g_path_is_absolute()
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
participants (2)
-
Luke Yue -
Michal Privoznik