Re: [Qemu-devel] [PATCH] mmap-alloc: check before use for ptr pointer

[Gonglei (Arei)]

> -----Original Message-----
> From: Michael Tokarev [mailto:address@hidden
> Sent: Wednesday, October 12, 2016 2:46 PM
> Subject: Re: [PATCH] mmap-alloc: check before use for ptr pointer
> 
> 12.10.2016 05:05, Gonglei wrote:
> > If ptr mmap failed, we don't need to do a superfluous
> > calculation for offset variable by ptr (MAP_FAILED).
> 
> What's the point?  There's no problem in extra calculation
> if mmap failed, yes, but do we really care?  As of it is now,
> it is more compact and readable, and works.  I think.
> 

I just think it's more reasonable because the ptr is checked after
used. Why do we need a extra calculation?


Regards,
-Gonglei

> Thanks,
> 
> /mjt
> 
> > Signed-off-by: Gonglei <address@hidden>
> > ---
> >  util/mmap-alloc.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> > index 5a85aa3..577862b 100644
> > --- a/util/mmap-alloc.c
> > +++ b/util/mmap-alloc.c
> > @@ -61,13 +61,15 @@ void *qemu_ram_mmap(int fd, size_t size, size_t
> align, bool shared)
> >  #else
> >      void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS |
> MAP_PRIVATE, -1, 0);
> >  #endif
> > -    size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> > +    size_t offset;
> >      void *ptr1;
> >
> >      if (ptr == MAP_FAILED) {
> >          return MAP_FAILED;
> >      }
> >
> > +    offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> > +
> >      /* Make sure align is a power of 2 */
> >      assert(!(align & (align - 1)));
> >      /* Always align to host page size */
> >