David Hildenbrand <da...@redhat.com> writes:
> On 30.11.19 20:42, Markus Armbruster wrote:
>> memory_device_get_free_addr() crashes when
>> memory_device_check_addable() fails and its @errp argument is null.
>> Messed up in commit 1b6d6af21b "pc-dimm: factor out capacity and slot
>> checks into MemoryDevice".
>>
>> The bug can't bite as no caller actually passes null. Fix it anyway.
>>
>> Cc: David Hildenbrand <da...@redhat.com>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> hw/mem/memory-device.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index aef148c1d7..4bc9cf0917 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -99,6 +99,7 @@ static uint64_t memory_device_get_free_addr(MachineState
>> *ms,
>> uint64_t align, uint64_t size,
>> Error **errp)
>> {
>> + Error *err = NULL;
>> GSList *list = NULL, *item;
>> Range as, new = range_empty;
>>
>> @@ -123,8 +124,9 @@ static uint64_t memory_device_get_free_addr(MachineState
>> *ms,
>> return 0;
>> }
>>
>> - memory_device_check_addable(ms, size, errp);
>> - if (*errp) {
>> + memory_device_check_addable(ms, size, &err);
>> + if (err) {
>> + error_propagate(errp, err);
>> return 0;
>> }
>
> I remember that some time ago, the best practice was to use "local_err",
> what is the latest state of that?
Hundreds of local Error * variables are named @local_err, and hundreds
more are named @err.
For what it's worth, the big comment in error.h uses @err, except in one
place where it needs two of them.
> I still disagree that these are BUGs or even latent BUGs. If somebody
> things these are BUGs and not cleanups, then we should probably have
> proper "Fixes: " tags instead.
Let's continue that discussion in the sub-thread where you first raised
this objection.
> Reviewed-by: David Hildenbrand <da...@redhat.com>
Thanks!
