From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:36795)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <nikunj@linux.vnet.ibm.com>) id 1VEBcC-0006t1-9K
	for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:14:59 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <nikunj@linux.vnet.ibm.com>) id 1VEBc4-0001Ac-Hg
	for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:14:52 -0400
Received: from e23smtp09.au.ibm.com ([202.81.31.142]:39114)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <nikunj@linux.vnet.ibm.com>) id 1VEBc3-0001AO-U8
	for qemu-devel@nongnu.org; Tue, 27 Aug 2013 01:14:44 -0400
Received: from /spool/local
	by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use
	Only! Violators will be prosecuted
	for <qemu-devel@nongnu.org> from <nikunj@linux.vnet.ibm.com>;
	Wed, 28 Aug 2013 02:08:51 +1000
From: Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
In-Reply-To: <421F6761-C72A-4BFD-9164-BFFB86E852A0@suse.de>
References: <1377249737-12570-1-git-send-email-aik@ozlabs.ru>
	<24C2B209-2082-4AF8-A8FB-1FF8A8B7751B@suse.de>
	<1377468637.3819.27.camel@pasglop>
	<87vc2tysur.fsf@linux.vnet.ibm.com>
	<F763F221-7BA8-4003-A2F9-7CBB822A519A@suse.de>
	<874naczpk5.fsf@linux.vnet.ibm.com>
	<011735F8-0ED7-40CF-A23A-CE34C21F1D3C@suse.de>
	<87wqn8y8ra.fsf@linux.vnet.ibm.com>
	<421F6761-C72A-4BFD-9164-BFFB86E852A0@suse.de>
Date: Tue, 27 Aug 2013 10:44:27 +0530
Message-ID: <8738pvbtqk.fsf@abhimanyu.in.ibm.com>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH] spapr-vscsi: Adding VSCSI capabilities
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alexander Graf <agraf@suse.de>
Cc: Alexey Kardashevskiy <aik@ozlabs.ru>, Paolo Bonzini <pbonzini@redhat.com>, "qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>, "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>

Alexander Graf <agraf@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