From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:38343)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Y0hlb-0001J5-CN
	for qemu-devel@nongnu.org; Mon, 15 Dec 2014 21:21:43 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Y0hlW-0007wm-LJ
	for qemu-devel@nongnu.org; Mon, 15 Dec 2014 21:21:39 -0500
Received: from [59.151.112.132] (port=20831 helo=heian.cn.fujitsu.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <yanghy@cn.fujitsu.com>) id 1Y0hlW-0007wP-9M
	for qemu-devel@nongnu.org; Mon, 15 Dec 2014 21:21:34 -0500
Message-ID: <548F9705.3010101@cn.fujitsu.com>
Date: Tue, 16 Dec 2014 10:20:53 +0800
From: Hongyang Yang <yanghy@cn.fujitsu.com>
MIME-Version: 1.0
References: <1418378027-19830-1-git-send-email-yanghy@cn.fujitsu.com>
In-Reply-To: <1418378027-19830-1-git-send-email-yanghy@cn.fujitsu.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: quoted-printable
Subject: Re: [Qemu-devel] [PATCH 1/2] QEMUSizedBuffer: only free qsb that
 qemu_bufopen allocated
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: qemu-devel@nongnu.org
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>

Ping!

=E5=9C=A8 12/12/2014 05:53 PM, Yang Hongyang =E5=86=99=E9=81=93:
> 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 <<open_mem_file_read>>
>              return qemu_bufopen("r", qsb);
> 2    404  tests/test-vmstate.c <<test_save_noskip>>
>              QEMUFile *fsave =3D qemu_bufopen("w", NULL);
> 3    424  tests/test-vmstate.c <<test_save_skip>>
>              QEMUFile *fsave =3D qemu_bufopen("w", NULL);
>
> Signed-off-by: Yang Hongyang <yanghy@cn.fujitsu.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Cc: Juan Quintela <quintela@redhat.com>
> ---
>   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 =3D 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, QEMUSizedB=
uffer *input)
>       }
>
>       s =3D g_malloc0(sizeof(QEMUBuffer));
> -    if (mode[0] =3D=3D 'r') {
> -        s->qsb =3D input;
> -    }
> +    s->qsb =3D input;
>
>       if (s->qsb =3D=3D NULL) {
>           s->qsb =3D qsb_create(NULL, 0);
> +        s->qsb_allocated =3D true;
>       }
>       if (!s->qsb) {
>           g_free(s);
>

--=20
Thanks,
Yang.