Re: [PATCH v3 21/22] qom/object: fix -Werror=maybe-uninitialized

[Markus Armbruster <armbru@redhat.com>]

marcandre.lureau@redhat.com writes:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> object_resolve_path_type() didn't always set *ambiguousp.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Fixes: 81c48dd79655 (hw/i386/acpi: Add object_resolve_type_unambiguous to improve modularity)

> ---
>  qom/object.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/qom/object.c b/qom/object.c
> index 28c5b66eab..bdc8a2c666 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -2201,6 +2201,9 @@ Object *object_resolve_path_type(const char *path, const char *typename,
>          }
>      } else {
>          obj = object_resolve_abs_path(object_get_root(), parts + 1, typename);
> +        if (ambiguousp) {
> +            *ambiguousp = false;
> +        }
>      }
>  
>      g_strfreev(parts);
> @@ -2226,7 +2229,7 @@ Object *object_resolve_path_at(Object *parent, const char *path)
>  
>  Object *object_resolve_type_unambiguous(const char *typename, Error **errp)
>  {
> -    bool ambig;
> +    bool ambig = false;
>      Object *o = object_resolve_path_type("", typename, &ambig);
>  
>      if (ambig) {

Contract:

   /**
    * object_resolve_path_type:
    * @path: the path to resolve
    * @typename: the type to look for.
    * @ambiguous: returns true if the path resolution failed because of an
    *   ambiguous match
    *
    * This is similar to object_resolve_path.  However, when looking for a
    * partial path only matches that implement the given type are considered.
    * This restricts the search and avoids spuriously flagging matches as
    * ambiguous.
    *
    * For both partial and absolute paths, the return value goes through
    * a dynamic cast to @typename.  This is important if either the link,
    * or the typename itself are of interface types.
    *
    * Returns: The matched object or NULL on path lookup failure.
    */

Note the parameter is called @ambiguous here, but @ambiguousp in the
definition.  Bad practice.

All the contract promises is that true will be stored in the variable
passed to @ambiguous when the function fails in a certain way.  For that
to work, the variable must be initialized to false.

You found a caller that doesn't: object_resolve_type_unambiguous().
This is a bug.  There might be more.  Impact is not obvious.

Two ways to fix:

1. Find all callers that don't, and fix them.  Your first hunk is then
   superfluous.  Your second hunk fixes the one you already found.

2. Change the contract so callers don't have to initialize.  Your second
   hunk is then superfluous.  The update to the contract is missing.

While there: the contract fails to specify that @ambiguous may be null.
Needs fixing, too.

Same for object_resolve_path().