Re: [Qemu-devel] [PATCH 05/18] qdev: provide a path resolution

[Anthony Liguori <anthony@codemonkey.ws>]

On 12/01/2011 04:24 AM, Stefan Hajnoczi wrote:
> On Wed, Nov 30, 2011 at 03:03:35PM -0600, Anthony Liguori wrote:
>> +DeviceState *qdev_resolve_path(const char *path, bool *ambiguous)
>> +{
>> +    bool partial_path = true;
>> +    DeviceState *dev;
>> +    gchar **parts;
>> +
>> +    parts = g_strsplit(path, "/", 0);
>> +    if (parts == NULL || parts[0] == NULL) {
>
> We must g_strfreev(parts) in the parts[0] == NULL case.

Good catch!

>
>> +/**
>> + * @qdev_resolve_path - resolves a path returning a device
>> + *
>> + * There are two types of supported paths--absolute paths and partial paths.
>> + *
>> + * Absolute paths are derived from the root device and can follow child<>  or
>> + * link<>  properties.  Since they can follow link<>  properties, they can be
>> + * arbitrarily long.  Absolute paths look like absolute filenames and are prefix
>
> s/prefix/prefixed/
>
>> + * with a leading slash.
>> + *
>> + * Partial paths are look like relative filenames.  They do not begin with a
>
> s/are//
>
>> + * prefix.  The matching rules for partial paths are subtle but designed to make
>> + * specifying devices easy.  At each level of the composition tree, the partial
>> + * path is matched as an absolute path.  The first match is not returned.  At
>> + * least two matches are searched for.  A successful result is only returned if
>> + * only one match is founded.  If more than one match is found, a flag is return
>
> s/return/returned/
>
>> + * to indicate that the match was ambiguous.
>> + *
>> + * @path - the path to resolve
>> + *
>> + * @ambiguous - returns true if the path resolution failed because of an
>> + *              ambiguous match
>
> The implementation seems to depend on ambiguous being initialized to
> false by the caller.  That would be worth documenting or changing so it
> does not depend on the initial value.

That's actually a bug.  I'll fix it.  Ack on the other comments too.

Regards,

Anthony Liguori

>
>> + *
>> + * Returns:
>> + *   The matched device.
>
> The matched device or NULL on path lookup failure.
>