Message ID | 1418378027-19830-1-git-send-email-yanghy@cn.fujitsu.com |
---|---|
State | New |
Headers | show |
Ping! 在 12/12/2014 05:53 PM, Yang Hongyang 写道: > 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 <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 = 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); >
* 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 <<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 <yanghy@cn.fujitsu.com> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> > Cc: Juan Quintela <quintela@redhat.com> Yes, this feels cleaner to me. Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> 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
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);
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 <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(-)