From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:56794)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1XnlAX-0003eH-H4
	for qemu-devel@nongnu.org; Mon, 10 Nov 2014 04:21:58 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1XnlAO-0004h0-AE
	for qemu-devel@nongnu.org; Mon, 10 Nov 2014 04:21:53 -0500
Received: from mx1.redhat.com ([209.132.183.28]:37073)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1XnlAO-0004gi-2S
	for qemu-devel@nongnu.org; Mon, 10 Nov 2014 04:21:44 -0500
Date: Mon, 10 Nov 2014 10:21:36 +0100
From: Kevin Wolf <kwolf@redhat.com>
Message-ID: <20141110092136.GC4158@noname.str.redhat.com>
References: <1415548218-25193-1-git-send-email-ming.lei@canonical.com>
	<1415548218-25193-2-git-send-email-ming.lei@canonical.com>
	<87lhnj31le.fsf@blackfin.pond.sub.org>
	<CACVXFVO9auQSFce+YiQEZtoir7KbCVqtSnvkE1JQZzBAXqtnYg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACVXFVO9auQSFce+YiQEZtoir7KbCVqtSnvkE1JQZzBAXqtnYg@mail.gmail.com>
Subject: Re: [Qemu-devel] [PATCH 1/2] virtio-scsi-dataplane: fix allocation
 for 'cmd_vrings'
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: Ming Lei <ming.lei@canonical.com>
Cc: Fam Zheng <famz@redhat.com>, Anthony Liguori <aliguori@amazon.com>, "Michael S. Tsirkin" <mst@redhat.com>, qemu-devel <qemu-devel@nongnu.org>, Markus Armbruster <armbru@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>

Am 10.11.2014 um 10:14 hat Ming Lei geschrieben:
> On Mon, Nov 10, 2014 at 4:24 PM, Markus Armbruster <armbru@redhat.com> wrote:
> > Ming Lei <ming.lei@canonical.com> writes:
> >
> >> The size of each element should be sizeof(VirtIOSCSIVring *).
> >>
> >> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> >> ---
> >>  hw/scsi/virtio-scsi-dataplane.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
> >> index 855439e..8a7cd9f 100644
> >> --- a/hw/scsi/virtio-scsi-dataplane.c
> >> +++ b/hw/scsi/virtio-scsi-dataplane.c
> >> @@ -239,7 +239,7 @@ void virtio_scsi_dataplane_start(VirtIOSCSI *s)
> >>      if (!s->event_vring) {
> >>          goto fail_vrings;
> >>      }
> >> -    s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring) * vs->conf.num_queues);
> >> +    s->cmd_vrings = g_malloc0(sizeof(VirtIOSCSIVring *) * vs->conf.num_queues);
> >>      for (i = 0; i < vs->conf.num_queues; i++) {
> >>          s->cmd_vrings[i] =
> >>              virtio_scsi_vring_init(s, vs->cmd_vqs[i],
> >
> > Please use something like
> >
> >     s->cmd_vrings = g_new0(VirtIOSCSIVring *, vs->conf.num_queues);
> >
> > This one crept in since I cleaned up g_malloc() use globally:
> 
> Your idea is good, but this one is a fix patch, and I
> think the g_new() conversion should be done in another
> patch since the two changes are different logically.

It's not really unrelated: g_new() would have caught the incorrect type
and made it a compiler error. So changing to g_new() in a patch fixing
such a bug is actually a logical conclusion and makes it more obvious
that your patch is correct.

Kevin