Alexander Graf <ag...@suse.de> writes:
>>>> + rc = spapr_vio_dma_read(&s->vdev, buffer, &cap, len);
>>>> + if (rc) {
>>>> + fprintf(stderr, "vscsi_send_capabilities: DMA read failure !\n");
>>>
>>> At this point cap contains random host data, no?
>>
>> Yes, and we zero it out in this case.
>
> Then please make this obvious to the reader. Either memset(0) it or do
> cap = { };. But do something that doesn't make me look up the header
> file to check whether you really did catch all the fields there are in
> the struct.
Sure
>
>>
>>>
>>>> + }
>>>> +
>>>> + /*
>>>> + * Current implementation does not suppport any migration or
>>>> + * reservation capabilities. Construct the response telling the
>>>> + * guest not to use them.
>>>> + */
>>>> + cap.flags = 0;
>>>> + cap.migration.ecl = 0;
>>>> + cap.reserve.type = 0;
>>>> + cap.migration.common.server_support = 0;
>>>> + cap.reserve.common.server_support = 0;
>>>> +
>>>> + rc = spapr_vio_dma_write(&s->vdev, buffer, &cap, len);
>>>> + if (rc) {
>>>> + fprintf(stderr, "vscsi_send_capabilities: DMA write failure !\n");
>>>> + }
>>>> +error_out:
>>>
>>> This is just "out" rather than "error_out", no? We also get here when we
>>> don't hit an error.
>>
>> Yes, the label is more readable at the goto, where we set the error
>> return code. In case of no error return code is H_SUCCESS. So that we
>> send a response back to the guest.
>
> ... which means we're not jumping into an error-only label.
True.
Regards
Nikunj
