[Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents

Alexey Kardashevskiy posted 2 patches 7 years, 6 months ago
There is a newer version of this series
[Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents
Posted by Alexey Kardashevskiy 7 years, 6 months ago
At the moment object_get_canonical_path_component() crashes on assert()
if the object does not have a parent. Usually it is not called for
orphan objects but various HMP/QMP commands can do that (info mtree,
qom-get).

This adds few more tests in object_get_canonical_path() to prevent QEMU
from crashing.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 qom/object.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qom/object.c b/qom/object.c
index 4677951..e0e300b 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1668,7 +1668,7 @@ gchar *object_get_canonical_path(Object *obj)
     Object *root = object_get_root();
     char *newpath, *path = NULL;
 
-    while (obj != root) {
+    while (obj && obj->parent && obj != root) {
         char *component = object_get_canonical_path_component(obj);
 
         if (path) {
-- 
2.11.0


Re: [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents
Posted by Paolo Bonzini 7 years, 6 months ago
On 30/04/2018 08:25, Alexey Kardashevskiy wrote:
> At the moment object_get_canonical_path_component() crashes on assert()
> if the object does not have a parent. Usually it is not called for
> orphan objects but various HMP/QMP commands can do that (info mtree,
> qom-get).
> 
> This adds few more tests in object_get_canonical_path() to prevent QEMU
> from crashing.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 4677951..e0e300b 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1668,7 +1668,7 @@ gchar *object_get_canonical_path(Object *obj)
>      Object *root = object_get_root();
>      char *newpath, *path = NULL;
>  
> -    while (obj != root) {
> +    while (obj && obj->parent && obj != root) {
>          char *component = object_get_canonical_path_component(obj);
>  
>          if (path) {

I think the patch is a good idea, but as it is written it is incorrect,
because it will return an invalid canonical path.  You should return
NULL instead.

Also, checking both obj and obj->parent is unnecessary; if obj->parent
is NULL, obj will be NULL on the next iteration.

Thanks,

Paolo