Re: [Qemu-trivial] [PATCH] qmp: Remove unused variable.

["Andreas Färber" <afaerber@suse.de>]

Am 25.04.2014 17:40, schrieb Luiz Capitulino:
> On Tue, 22 Apr 2014 22:44:03 +0100
> Hani Benhabiles <kroosec@gmail.com> wrote:
> 
>> Signed-off-by: Hani Benhabiles <hani@linux.com>
>> ---
>>  qmp.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/qmp.c b/qmp.c
>> index 87a28f7..44a6e17 100644
>> --- a/qmp.c
>> +++ b/qmp.c
>> @@ -194,11 +194,10 @@ void qmp_system_wakeup(Error **errp)
>>  ObjectPropertyInfoList *qmp_qom_list(const char *path, Error **errp)
>>  {
>>      Object *obj;
>> -    bool ambiguous = false;
>>      ObjectPropertyInfoList *props = NULL;
>>      ObjectProperty *prop;
>>  
>> -    obj = object_resolve_path(path, &ambiguous);
>> +    obj = object_resolve_path(path, NULL);
>>      if (obj == NULL) {
>>          error_set(errp, QERR_DEVICE_NOT_FOUND, path);
>>          return NULL;
> 
> I'm under the impression that this check in object_resolve_partial_path():
> 
>         if (ambiguous && *ambiguous) {
>             return NULL;
>         }
> 
> Uses 'ambiguous' internally. In that case, this change could have a side effect.
> 
> But I'm not sure, I think it would be good to get a reviewed-by from
> a QOM expert.

Your understanding matches mine. I would propose to instead use the
variable in error reporting:

if (ambiguous) {
    error_setg(errp, "Path '%s' is ambiguous.", path);
} else {
    error_set(errp, QERR_DEVICE_NOT_FOUND, path);
}

My reasoning is that an ambiguous path might deliver unpredictable
results (whichever it encounters first), across QEMU versions at least.

But I'd like to hear Paolo's opinion, too, since he was involved in
loosening requirements on paths.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg