| Message ID | 20170207132723.13934-2-famz@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 02/07/17 14:27, Fam Zheng wrote: > Laszlo spotted and studied this wasteful "if". He pointed out: > > The original virtio_blk_free_request needed an "if" as it accesses one > field, since 671ec3f05655 ("virtio-blk: Convert VirtIOBlockReq.elem to > pointer", 2014-06-11); later on in f897bf751fbd ("virtio-blk: embed > VirtQueueElement in VirtIOBlockReq", 2014-07-09) the field became > embedded, so the "if" became unnecessary (at which point we were using > g_slice_free(), but it is the same. > > Now drop it. > > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > hw/block/virtio-blk.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 702eda8..2858c31 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -42,9 +42,7 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, > > static void virtio_blk_free_request(VirtIOBlockReq *req) > { > - if (req) { > - g_free(req); > - } > + g_free(req); > } > > static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > Reviewed-by: Laszlo Ersek <lersek@redhat.com> Thanks! Laszlo
On Tue, Feb 07, 2017 at 09:27:22PM +0800, Fam Zheng wrote: > Laszlo spotted and studied this wasteful "if". He pointed out: > > The original virtio_blk_free_request needed an "if" as it accesses one > field, since 671ec3f05655 ("virtio-blk: Convert VirtIOBlockReq.elem to > pointer", 2014-06-11); later on in f897bf751fbd ("virtio-blk: embed > VirtQueueElement in VirtIOBlockReq", 2014-07-09) the field became > embedded, so the "if" became unnecessary (at which point we were using > g_slice_free(), but it is the same. > > Now drop it. > > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > hw/block/virtio-blk.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
On Tue, 02/07 21:27, Fam Zheng wrote: > Laszlo spotted and studied this wasteful "if". He pointed out: > > The original virtio_blk_free_request needed an "if" as it accesses one > field, since 671ec3f05655 ("virtio-blk: Convert VirtIOBlockReq.elem to > pointer", 2014-06-11); later on in f897bf751fbd ("virtio-blk: embed > VirtQueueElement in VirtIOBlockReq", 2014-07-09) the field became > embedded, so the "if" became unnecessary (at which point we were using > g_slice_free(), but it is the same. > > Now drop it. > > Reported-by: Laszlo Ersek <lersek@redhat.com> > Signed-off-by: Fam Zheng <famz@redhat.com> > --- > hw/block/virtio-blk.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 702eda8..2858c31 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -42,9 +42,7 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, > > static void virtio_blk_free_request(VirtIOBlockReq *req) > { > - if (req) { > - g_free(req); > - } > + g_free(req); > } > > static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) > -- > 2.9.3 > > Cc: qemu-trivial@nongnu.org (Let's drop 2/2 and perhaps merge this via trivial) Fam
07.02.2017 16:27, Fam Zheng wrote: > Laszlo spotted and studied this wasteful "if". He pointed out: > > The original virtio_blk_free_request needed an "if" as it accesses one > field, since 671ec3f05655 ("virtio-blk: Convert VirtIOBlockReq.elem to > pointer", 2014-06-11); later on in f897bf751fbd ("virtio-blk: embed > VirtQueueElement in VirtIOBlockReq", 2014-07-09) the field became > embedded, so the "if" became unnecessary (at which point we were using > g_slice_free(), but it is the same. > > Now drop it. Applied to -trivial, thanks! /mjt
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 702eda8..2858c31 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -42,9 +42,7 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, static void virtio_blk_free_request(VirtIOBlockReq *req) { - if (req) { - g_free(req); - } + g_free(req); } static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status)
Laszlo spotted and studied this wasteful "if". He pointed out: The original virtio_blk_free_request needed an "if" as it accesses one field, since 671ec3f05655 ("virtio-blk: Convert VirtIOBlockReq.elem to pointer", 2014-06-11); later on in f897bf751fbd ("virtio-blk: embed VirtQueueElement in VirtIOBlockReq", 2014-07-09) the field became embedded, so the "if" became unnecessary (at which point we were using g_slice_free(), but it is the same. Now drop it. Reported-by: Laszlo Ersek <lersek@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> --- hw/block/virtio-blk.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)