[Qemu-devel] [PATCH v2 0/4] Fixes around qemu_vfree()

[Qemu-devel] [PATCH v2 0/4] Fixes around qemu_vfree()

[Markus Armbruster]

v2: Fix silly mistake in 4/4; the other three patches are unchanged

Markus Armbruster (4):
  w32: Make qemu_vfree() accept NULL like the POSIX implementation
  scsi-disk: qemu_vfree(NULL) is fine, simplify
  win32-aio: Fix how win32_aio_process_completion() frees buffer
  block: Fix how mirror_run() frees its buffer

 block/mirror.c     | 2 +-
 block/win32-aio.c  | 2 +-
 hw/scsi-disk.c     | 4 +---
 util/oslib-win32.c | 4 +++-
 4 files changed, 6 insertions(+), 6 deletions(-)

-- 
1.7.11.7

[Qemu-devel] [PATCH v2 1/4] w32: Make qemu_vfree() accept NULL like the POSIX implementation

[Markus Armbruster]

On POSIX, qemu_vfree() accepts NULL, because it's merely wrapper
around free().  As far as I can tell, the Windows implementation
doesn't.  Breeds bugs that bite only under Windows.

Make the Windows implementation behave like the POSIX implementation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 util/oslib-win32.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e7e283e..640194c 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -71,7 +71,9 @@ void *qemu_vmalloc(size_t size)
 void qemu_vfree(void *ptr)
 {
     trace_qemu_vfree(ptr);
-    VirtualFree(ptr, 0, MEM_RELEASE);
+    if (ptr) {
+        VirtualFree(ptr, 0, MEM_RELEASE);
+    }
 }
 
 /* FIXME: add proper locking */
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 2/4] scsi-disk: qemu_vfree(NULL) is fine, simplify

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/scsi-disk.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/scsi-disk.c b/hw/scsi-disk.c
index f8d7ef3..96db9a7 100644
--- a/hw/scsi-disk.c
+++ b/hw/scsi-disk.c
@@ -85,9 +85,7 @@ static void scsi_free_request(SCSIRequest *req)
 {
     SCSIDiskReq *r = DO_UPCAST(SCSIDiskReq, req, req);
 
-    if (r->iov.iov_base) {
-        qemu_vfree(r->iov.iov_base);
-    }
+    qemu_vfree(r->iov.iov_base);
 }
 
 /* Helper function for command completion with sense.  */
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 3/4] win32-aio: Fix how win32_aio_process_completion() frees buffer

[Markus Armbruster]

win32_aio_submit() allocates it with qemu_blockalign(), therefore it
must be freed with qemu_vfree(), not g_free().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/win32-aio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/win32-aio.c b/block/win32-aio.c
index 46a5db7..0383370 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -87,7 +87,7 @@ static void win32_aio_process_completion(QEMUWin32AIOState *s,
                 memcpy(p, qiov->iov[i].iov_base, qiov->iov[i].iov_len);
                 p += qiov->iov[i].iov_len;
             }
-            g_free(waiocb->buf);
+            qemu_vfree(waiocb->buf);
         }
     }
 
-- 
1.7.11.7

[Qemu-devel] [PATCH v2 4/4] block: Fix how mirror_run() frees its buffer

[Markus Armbruster]

It allocates with qemu_blockalign(), therefore it must free with
qemu_vfree(), not g_free().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/mirror.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 8aeacbf..6180aa3 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -225,7 +225,7 @@ static void coroutine_fn mirror_run(void *opaque)
     }
 
 immediate_exit:
-    g_free(s->buf);
+    qemu_vfree(s->buf);
     bdrv_set_dirty_tracking(bs, false);
     bdrv_iostatus_disable(s->target);
     if (s->should_complete && ret == 0) {
-- 
1.7.11.7

Re: [Qemu-devel] [PATCH v2 0/4] Fixes around qemu_vfree()

[Kevin Wolf]

Am 15.01.2013 15:29, schrieb Markus Armbruster:
> v2: Fix silly mistake in 4/4; the other three patches are unchanged
> 
> Markus Armbruster (4):
>   w32: Make qemu_vfree() accept NULL like the POSIX implementation
>   scsi-disk: qemu_vfree(NULL) is fine, simplify
>   win32-aio: Fix how win32_aio_process_completion() frees buffer
>   block: Fix how mirror_run() frees its buffer
> 
>  block/mirror.c     | 2 +-
>  block/win32-aio.c  | 2 +-
>  hw/scsi-disk.c     | 4 +---
>  util/oslib-win32.c | 4 +++-
>  4 files changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/4] Fixes around qemu_vfree()

[Stefan Hajnoczi]

On Tue, Jan 15, 2013 at 03:35:43PM +0100, Kevin Wolf wrote:
> Am 15.01.2013 15:29, schrieb Markus Armbruster:
> > v2: Fix silly mistake in 4/4; the other three patches are unchanged
> > 
> > Markus Armbruster (4):
> >   w32: Make qemu_vfree() accept NULL like the POSIX implementation
> >   scsi-disk: qemu_vfree(NULL) is fine, simplify
> >   win32-aio: Fix how win32_aio_process_completion() frees buffer
> >   block: Fix how mirror_run() frees its buffer
> > 
> >  block/mirror.c     | 2 +-
> >  block/win32-aio.c  | 2 +-
> >  hw/scsi-disk.c     | 4 +---
> >  util/oslib-win32.c | 4 +++-
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Thanks, applied Patch 4 to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan