Re: [kvm-unit-tests PATCH 02/18] trivial: lib: fail hard on failed mallocs

[Andrew Jones]

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 
> > ---
> >  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 majord...@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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [kvm-unit-tests PATCH 02/18] trivial: lib: fail hard on failed mallocs

[Thomas Huth]

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 
> ---
>  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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html