[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] QEMUSizedBuffer: only free qsb that qemu_bu
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] QEMUSizedBuffer: only free qsb that qemu_bufopen allocated |
Date: |
Tue, 16 Dec 2014 13:34:08 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Yang Hongyang (address@hidden) 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 <<open_mem_file_read>>
> return qemu_bufopen("r", qsb);
> 2 404 tests/test-vmstate.c <<test_save_noskip>>
> QEMUFile *fsave = qemu_bufopen("w", NULL);
> 3 424 tests/test-vmstate.c <<test_save_skip>>
> QEMUFile *fsave = qemu_bufopen("w", NULL);
>
> Signed-off-by: Yang Hongyang <address@hidden>
> Cc: Dr. David Alan Gilbert <address@hidden>
> Cc: Juan Quintela <address@hidden>
Yes, this feels cleaner to me.
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
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 / address@hidden / Manchester, UK