| Message ID | 20180530162349.31100-1-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qom: support orphan objects in object_get_canonical_path | expand |
On 05/30/2018 01:23 PM, Paolo Bonzini wrote: > Mostly a rewrite, in order to keep the loop simple. Thus easier to review using "git difftool -y -x sdiff" (or whichever tool you prefer, such meld). "git format-patch" doesn't provide a way to generate side-by-side diffs applicable. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qom/object.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 0fc972030e..4f30431ae3 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1669,25 +1669,28 @@ gchar *object_get_canonical_path(Object *obj) > Object *root = object_get_root(); > char *newpath, *path = NULL; > > - while (obj != root) { > + if (obj == root) { > + return g_strdup("/"); > + } > + > + do { > char *component = object_get_canonical_path_component(obj); > > - if (path) { > - newpath = g_strdup_printf("%s/%s", component, path); > - g_free(component); > + if (!component) { > + /* A canonical path must be complete, so discard what was > + * collected so far. > + */ > g_free(path); > - path = newpath; > - } else { > - path = component; > + return NULL; This function now matches his documentation! > } > > - obj = obj->parent; > - } > - > - newpath = g_strdup_printf("/%s", path ? path : ""); > - g_free(path); > + newpath = g_strdup_printf("/%s%s", component, path ? path : ""); > + g_free(path); > + g_free(component); > + path = newpath; Easier to read in 2 lines: obj = obj->parent; } while (obj != root); > + } while ((obj = obj->parent) != root); > > - return newpath; > + return path; > } > > Object *object_resolve_path_component(Object *parent, const gchar *part) > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 31/5/18 2:23 am, Paolo Bonzini wrote: > Mostly a rewrite, in order to keep the loop simple. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > qom/object.c | 29 ++++++++++++++++------------- > 1 file changed, 16 insertions(+), 13 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 0fc972030e..4f30431ae3 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1669,25 +1669,28 @@ gchar *object_get_canonical_path(Object *obj) > Object *root = object_get_root(); > char *newpath, *path = NULL; > > - while (obj != root) { > + if (obj == root) { > + return g_strdup("/"); > + } > + > + do { > char *component = object_get_canonical_path_component(obj); > > - if (path) { > - newpath = g_strdup_printf("%s/%s", component, path); > - g_free(component); > + if (!component) { > + /* A canonical path must be complete, so discard what was > + * collected so far. > + */ Well, this is correct indeed for the normal case when the result is used for internal business but for my task (show the owner of an MR or at least give a clue what to grep for) it will discard a partial path. I guess I could print a typename if object_get_canonical_path() returns NULL, I'll repost v4. > g_free(path); > - path = newpath; > - } else { > - path = component; > + return NULL; > } > > - obj = obj->parent; > - } > - > - newpath = g_strdup_printf("/%s", path ? path : ""); > - g_free(path); > + newpath = g_strdup_printf("/%s%s", component, path ? path : ""); > + g_free(path); > + g_free(component); > + path = newpath; > + } while ((obj = obj->parent) != root); > > - return newpath; > + return path; > } > > Object *object_resolve_path_component(Object *parent, const gchar *part) >
On 31/05/2018 05:45, Alexey Kardashevskiy wrote: > Well, this is correct indeed for the normal case when the result is used > for internal business but for my task (show the owner of an MR or at least > give a clue what to grep for) it will discard a partial path. > > I guess I could print a typename if object_get_canonical_path() returns > NULL, I'll repost v4. Very good! Paolo
diff --git a/qom/object.c b/qom/object.c index 0fc972030e..4f30431ae3 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1669,25 +1669,28 @@ gchar *object_get_canonical_path(Object *obj) Object *root = object_get_root(); char *newpath, *path = NULL; - while (obj != root) { + if (obj == root) { + return g_strdup("/"); + } + + do { char *component = object_get_canonical_path_component(obj); - if (path) { - newpath = g_strdup_printf("%s/%s", component, path); - g_free(component); + if (!component) { + /* A canonical path must be complete, so discard what was + * collected so far. + */ g_free(path); - path = newpath; - } else { - path = component; + return NULL; } - obj = obj->parent; - } - - newpath = g_strdup_printf("/%s", path ? path : ""); - g_free(path); + newpath = g_strdup_printf("/%s%s", component, path ? path : ""); + g_free(path); + g_free(component); + path = newpath; + } while ((obj = obj->parent) != root); - return newpath; + return path; } Object *object_resolve_path_component(Object *parent, const gchar *part)
Mostly a rewrite, in order to keep the loop simple. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- qom/object.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)