[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as th
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name |
Date: |
Wed, 20 Aug 2014 15:04:32 +1000 |
On Wed, Aug 20, 2014 at 5:01 AM, Peter Maydell <address@hidden> wrote:
> On 19 August 2014 11:43, Paolo Bonzini <address@hidden> wrote:
>> From: Peter Crosthwaite <address@hidden>
>>
>> Rather than having the name as separate state. This prepares support
>> for creating a MemoryRegion dynamically (i.e. without
>> memory_region_init() and friends) and the MemoryRegion still getting
>> a usable name.
>> @@ -1310,7 +1308,7 @@ uint64_t memory_region_size(MemoryRegion *mr)
>>
>> const char *memory_region_name(const MemoryRegion *mr)
>> {
>> - return mr->name;
>> + return object_get_canonical_path_component(OBJECT(mr));
>> }
>
> This doesn't look right. It changes the semantics of this function
> from "returns a string which you don't own and can't change
> but don't need to free" to "returns a copy of a string which you
> have to free with g_free() when you're done". Unsurprisingly,
> the callsites aren't expecting this and we leak the string all
> over the place.
>
> I think we need to revert this (commit b0225c2c0d8) until
> both the Xen callsites are fixed and the leak issue is
> dealt with.
>
Have half a plan on the leak issue. With
object_get_canonical_path_component() being used increasing as a "get
me the already set, name of this object", I think it's better to just
change to API to return const and remove the free burden from all call
sites completely. If call sites really want a fresh copy they can take
one themselves. Patch on list imminently.
Regards,
Peter
> thanks
> -- PMM
>
- [Qemu-devel] [PULL 13/20] memory: remove memory_region_destroy, (continued)
- [Qemu-devel] [PULL 13/20] memory: remove memory_region_destroy, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 14/20] tpm_tis: remove instance_finalize callback, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 15/20] loader: Abstract away ref to memory region names, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 16/20] exec: Abstract away ref to memory region names, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 17/20] memory: constify memory_region_name, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 18/20] memory: Use memory_region_name for name access, Paolo Bonzini, 2014/08/19
- [Qemu-devel] [PULL 19/20] memory: Use canonical path component as the name, Paolo Bonzini, 2014/08/19
[Qemu-devel] [PULL 20/20] mtree: remove write-only field, Paolo Bonzini, 2014/08/19
Re: [Qemu-devel] [PULL 00/20] SCSI and memory changes for 2014-08-18, Peter Maydell, 2014/08/19