Re: [Qemu-block] [Qemu-devel] [PATCH] hw/block/fdc: floppy command FIFO memory initialization

[John Snow]

On 5/29/19 9:56 AM, Andrey Shinkevich wrote:
> 
> 
> On 29/05/2019 16:40, John Snow wrote:
>>
>>
>> On 5/29/19 8:22 AM, Andrey Shinkevich wrote:
>>> The uninitialized memory allocated for the command FIFO of the
>>> floppy controller during the VM hardware initialization incurs
>>> many unwanted reports by Valgrind when VM state is being saved.
>>> That verbosity hardens a search for the real memory issues when
>>> the iotests run. Particularly, the patch eliminates 20 unnecessary
>>> reports of the Valgrind tool in the iotest #169.
>>>
>>> Signed-off-by: Andrey Shinkevich <address@hidden>
>>> ---
>>>   hw/block/fdc.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>>> index 6f19f12..54e470c 100644
>>> --- a/hw/block/fdc.c
>>> +++ b/hw/block/fdc.c
>>> @@ -2647,6 +2647,10 @@ static void fdctrl_realize_common(DeviceState *dev, 
>>> FDCtrl *fdctrl,
>>>   
>>>       FLOPPY_DPRINTF("init controller\n");
>>>       fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
>>> +    if (fdctrl->fifo) {
>>> +        /* To avoid using the uninitialized memory while saving VM state */
>>> +        memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
>>> +    }
>>
>> qemu_memalign doesn't look like it can fail (looking at
>> util/oslib-posix); is this conditional necessary?
>>
>> I think you could just:
>>
>> fdctrl->fifo = qemu_memalign(512, FD_SECTOR_LEN);
>> memset(fdctrl->fifo, 0, FD_SECTOR_LEN);
>>
>>>       fdctrl->fifo_size = 512;
>>>       fdctrl->result_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
>>>                                                fdctrl_result_timer, fdctrl);
>>>
> 
> Yes, that's right.
> Thank you, John.
> 
> Andrey
> 

Thanks for valgrinding QEMU :)

--js