From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53283) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0sGc-0000oB-5m for qemu-devel@nongnu.org; Tue, 16 Dec 2014 08:34:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y0sGX-0003Q6-Hq for qemu-devel@nongnu.org; Tue, 16 Dec 2014 08:34:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41569) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y0sGX-0003Q1-A8 for qemu-devel@nongnu.org; Tue, 16 Dec 2014 08:34:17 -0500 Date: Tue, 16 Dec 2014 13:34:08 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20141216133407.GE5467@work-vm> References: <1418378027-19830-1-git-send-email-yanghy@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418378027-19830-1-git-send-email-yanghy@cn.fujitsu.com> Subject: Re: [Qemu-devel] [PATCH 1/2] QEMUSizedBuffer: only free qsb that qemu_bufopen allocated List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Yang Hongyang Cc: amit.shah@redhat.com, qemu-devel@nongnu.org, Juan Quintela * Yang Hongyang (yanghy@cn.fujitsu.com) wrote: > Only free qsb that qemu_bufopen allocated, and also allow > qemu_bufopen accept qsb as input for write operation. It > will make the API more logical: > 1.If you create the QEMUSizedBuffer yourself, you need to > free it by using qsb_free() but not depends on other API > like qemu_fclose. > 2.allow qemu_bufopen() accept QEMUSizedBuffer as input for > write operation, otherwise, it will be a little strange > for this API won't accept the second parameter. > > This brings API change, since there are only 3 > users of this API currently, this change only impact the > first one which will be fixed in patch 2 of this patchset, > so I think it is safe to do this change. > > 1 70 tests/test-vmstate.c <> > return qemu_bufopen("r", qsb); > 2 404 tests/test-vmstate.c <> > QEMUFile *fsave = qemu_bufopen("w", NULL); > 3 424 tests/test-vmstate.c <> > QEMUFile *fsave = qemu_bufopen("w", NULL); > > Signed-off-by: Yang Hongyang > Cc: Dr. David Alan Gilbert > Cc: Juan Quintela Yes, this feels cleaner to me. Reviewed-by: Dr. David Alan Gilbert Dave > --- > qemu-file.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/qemu-file.c b/qemu-file.c > index f938e36..52f8d69 100644 > --- a/qemu-file.c > +++ b/qemu-file.c > @@ -904,6 +904,7 @@ QEMUSizedBuffer *qsb_clone(const QEMUSizedBuffer *qsb) > typedef struct QEMUBuffer { > QEMUSizedBuffer *qsb; > QEMUFile *file; > + bool qsb_allocated; > } QEMUBuffer; > > static int buf_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size) > @@ -933,7 +934,9 @@ static int buf_close(void *opaque) > { > QEMUBuffer *s = opaque; > > - qsb_free(s->qsb); > + if (s->qsb_allocated) { > + qsb_free(s->qsb); > + } > > g_free(s); > > @@ -972,12 +975,11 @@ QEMUFile *qemu_bufopen(const char *mode, QEMUSizedBuffer *input) > } > > s = g_malloc0(sizeof(QEMUBuffer)); > - if (mode[0] == 'r') { > - s->qsb = input; > - } > + s->qsb = input; > > if (s->qsb == NULL) { > s->qsb = qsb_create(NULL, 0); > + s->qsb_allocated = true; > } > if (!s->qsb) { > g_free(s); > -- > 1.9.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK