On Wed, Aug 20, 2014 at 7:07 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 20 August 2014 06:08, Peter Crosthwaite <peter.crosthwa...@xilinx.com>
> wrote:
>> With information about return value ownership.
>>
>> Signed-off-by: Peter Crosthwaite <peter.crosthwa...@xilinx.com>
>> ---
>>
>> 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.
Regards,
Peter
> If we do do this I think I'd put the doc comment change in
> the same patch that changes the semantics.
>
> thanks
> -- PMM
>
