| Message ID | 1446769483-21586-3-git-send-email-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/11/15 01:24, Andrew Jones wrote: > It's pretty safe to not even bother checking for NULL when > using malloc and friends, but if we do check, then fail > hard. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > lib/virtio-mmio.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c > index 043832299174e..1b6f0cc378b79 100644 > --- a/lib/virtio-mmio.c > +++ b/lib/virtio-mmio.c ... > @@ -161,9 +160,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid) > if (node == -FDT_ERR_NOTFOUND) > return NULL; > > - vm_dev = calloc(1, sizeof(*vm_dev)); > - if (!vm_dev) > - return NULL; > + assert((vm_dev = calloc(1, sizeof(*vm_dev))) != NULL); Could you maybe write that as two statements? assert() is classically a macro which could also be disabled, so if somebody introduces a switch to "#define assert(...) /*nothing*/" in the future, that calloc call would be completely gone! Thomas -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 06, 2015 at 03:05:41PM +0100, Thomas Huth wrote: > On 06/11/15 01:24, Andrew Jones wrote: > > It's pretty safe to not even bother checking for NULL when > > using malloc and friends, but if we do check, then fail > > hard. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > --- > > lib/virtio-mmio.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c > > index 043832299174e..1b6f0cc378b79 100644 > > --- a/lib/virtio-mmio.c > > +++ b/lib/virtio-mmio.c > ... > > @@ -161,9 +160,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid) > > if (node == -FDT_ERR_NOTFOUND) > > return NULL; > > > > - vm_dev = calloc(1, sizeof(*vm_dev)); > > - if (!vm_dev) > > - return NULL; > > + assert((vm_dev = calloc(1, sizeof(*vm_dev))) != NULL); > > Could you maybe write that as two statements? assert() is classically a > macro which could also be disabled, so if somebody introduces a switch > to "#define assert(...) /*nothing*/" in the future, that calloc call > would be completely gone! That's a good point, and I'm happy to change this one. Unfortunately I've done things like this several times before, so if we decide to allow assert to be a noop, then we'll need to change those other places as well. I think it's pretty unlikely we ever will want to make it a noop for kvm-unit-tests though. Thanks, drew > > Thomas > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/virtio-mmio.c b/lib/virtio-mmio.c index 043832299174e..1b6f0cc378b79 100644 --- a/lib/virtio-mmio.c +++ b/lib/virtio-mmio.c @@ -54,8 +54,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device *vdev, vq = calloc(1, sizeof(*vq)); queue = memalign(PAGE_SIZE, VIRTIO_MMIO_QUEUE_SIZE_MIN); - if (!vq || !queue) - return NULL; + assert(vq && queue); writel(index, vm_dev->base + VIRTIO_MMIO_QUEUE_SEL); @@ -161,9 +160,7 @@ static struct virtio_device *virtio_mmio_dt_bind(u32 devid) if (node == -FDT_ERR_NOTFOUND) return NULL; - vm_dev = calloc(1, sizeof(*vm_dev)); - if (!vm_dev) - return NULL; + assert((vm_dev = calloc(1, sizeof(*vm_dev))) != NULL); vm_dev->base = info.base; vm_device_init(vm_dev);
It's pretty safe to not even bother checking for NULL when using malloc and friends, but if we do check, then fail hard. Signed-off-by: Andrew Jones <drjones@redhat.com> --- lib/virtio-mmio.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)