On Fri, 7 Jul 2017 18:30:52 -0300
Eduardo Habkost <ehabk...@redhat.com> wrote:
> object_resolve_path*() ambiguous path detection breaks when
> ambiguous==NULL and the object tree have 3 objects of the same type and
> only 2 of them are under the same parent. e.g.:
>
> /container/obj1 (TYPE_FOO)
> /container/obj2 (TYPE_FOO)
> /obj2 (TYPE_FOO)
>
> With the above tree, object_resolve_path_type("", TYPE_FOO, NULL) will
> incorrectly return /obj2, because the search inside "/container" will
> return NULL, and the match at "/obj2" won't be detected as ambiguous.
>
> Fix that by always calling object_resolve_partial_path() with a non-NULL
> ambiguous parameter.
>
> Test case included.
>
> Reported-by: Igor Mammedov <imamm...@redhat.com>
> Cc: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> qom/object.c | 17 ++++++++---------
> tests/check-qom-proplist.c | 3 +++
> 2 files changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 5f6fdfa..0cdddcb 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -1712,15 +1712,13 @@ static Object *object_resolve_partial_path(Object
> *parent,
> typename, ambiguous);
> if (found) {
> if (obj) {
> - if (ambiguous) {
> - *ambiguous = true;
> - }
> + *ambiguous = true;
> return NULL;
> }
> obj = found;
> }
>
> - if (ambiguous && *ambiguous) {
> + if (*ambiguous) {
> return NULL;
> }
> }
> @@ -1729,7 +1727,7 @@ static Object *object_resolve_partial_path(Object
> *parent,
> }
>
> Object *object_resolve_path_type(const char *path, const char *typename,
> - bool *ambiguous)
> + bool *ambiguousp)
> {
> Object *obj;
> gchar **parts;
> @@ -1738,11 +1736,12 @@ Object *object_resolve_path_type(const char *path,
> const char *typename,
> assert(parts);
>
> if (parts[0] == NULL || strcmp(parts[0], "") != 0) {
> - if (ambiguous) {
> - *ambiguous = false;
> - }
> + bool ambiguous = false;
> obj = object_resolve_partial_path(object_get_root(), parts,
> - typename, ambiguous);
> + typename, &ambiguous);
> + if (ambiguousp) {
> + *ambiguousp = ambiguous;
> + }
> } else {
> obj = object_resolve_abs_path(object_get_root(), parts, typename, 1);
> }
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index abafbd7..381532c 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -593,14 +593,17 @@ static void test_qom_partial_path(void)
> ambiguous = false;
> g_assert(!object_resolve_path_type("", TYPE_DUMMY, &ambiguous));
> g_assert(ambiguous);
> + g_assert(!object_resolve_path_type("", TYPE_DUMMY, NULL));
>
> ambiguous = false;
> g_assert(!object_resolve_path("obj2", &ambiguous));
> g_assert(ambiguous);
> + g_assert(!object_resolve_path("obj2", NULL));
>
> ambiguous = false;
> g_assert(object_resolve_path("obj1", &ambiguous) == obj1);
> g_assert(!ambiguous);
> + g_assert(object_resolve_path("obj1", NULL) == obj1);
>
> object_unparent(obj1);
> object_unparent(obj2a);
