[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH bugfix v1 3/3] qom: object.h: Update object_get_

From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH bugfix v1 3/3] qom: object.h: Update object_get_canon_path* doc
Date: Thu, 21 Aug 2014 00:54:22 +1000

On Wed, Aug 20, 2014 at 7:07 PM, Peter Maydell <address@hidden> wrote:
> On 20 August 2014 06:08, Peter Crosthwaite <address@hidden> wrote:
>> With information about return value ownership.
>> Signed-off-by: Peter Crosthwaite <address@hidden>
>> ---
>>  include/qom/object.h | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/include/qom/object.h b/include/qom/object.h
>> index 8618e49..87de889 100644
>> --- a/include/qom/object.h
>> +++ b/include/qom/object.h
>> @@ -1029,7 +1029,8 @@ Object *object_get_root(void);
>>   * object_get_canonical_path_component:
>>   *
>>   * Returns: The final component in the object's canonical path.  The 
>> canonical
>> - * path is the path within the composition tree starting from the root.
>> + * path is the path within the composition tree starting from the root. The
>> + * returned value may not be modified.
>>   */
>>  const gchar *object_get_canonical_path_component(Object *obj);
> The other thing you need to say is that the returned string is
> only valid for as long as the object remains a child property
> of its parent. (Is that right? I'm not clear. It also sounds like
> a really awkward lifetime management requirement, which
> suggests to me that really the "return-a-copy" semantics are
> nicer.)

It is. But in QOM that is the naming scheme. An object is uniquely
identified by it's full canonical path. If you don't have a parent,
you don't have a name. If you have a dup of the name, but then the
object gets reparented or destroyed your name is stale and I think the
caller should have to deal with that.

> Having object_get_canonical_path_component() and
> object_get_canonical_path() having different return value
> ownership semantics is likely to be a bit confusing I think.

Alternatives I can think of include,

1: All memory_region_name() callsites have to do a free.
2: Memory API keeps a single copy of the name (lazily inited in
memory_region_name perhaps).
3: Patch struct object with a char instance_name[] field and have QOM
ensure the fixed location is always a valid name ("\0" for an
unparented object).
4: This

Please let me know your preference.


> If we do do this I think I'd put the doc comment change in
> the same patch that changes the semantics.
> thanks
> -- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]