[Beignet] [PATCH] Runtime: Use cl_ulong as CL_DEVICE_MAX_MEM_ALLOC_SIZE's return type.

[Yang, Rong R]

> -----Original Message-----
> From: Beignet [mailto:beignet-bounces at lists.freedesktop.org] On Behalf Of
> Simon Richter
> Sent: Thursday, December 8, 2016 20:36
> To: beignet at lists.freedesktop.org
> Subject: Re: [Beignet] [PATCH] Runtime: Use cl_ulong as
> CL_DEVICE_MAX_MEM_ALLOC_SIZE's return type.
> 
> Hi,
> 
> On Thu, Dec 08, 2016 at 03:47:28PM +0800, Yang Rong wrote:
> 
> > diff --git a/src/cl_device_id.c b/src/cl_device_id.c index
> > 24334fd..71a7be1 100644
> > --- a/src/cl_device_id.c
> > +++ b/src/cl_device_id.c
> > @@ -926,6 +926,7 @@ cl_get_device_ids(cl_platform_id    platform,
> >      }                                                               \
> >      if (param_value_size < sizeof device->FIELD)                    \
> >        return CL_INVALID_VALUE;                                      \
> > +    memset(param_value, 0, param_value_size);                       \
> >      memcpy(param_value, &device->FIELD, sizeof device->FIELD);      \
> >      return CL_SUCCESS;
> >
> 
> I don't see the point -- programs are not supposed to behave differently
> here, and it might hide errors when running under valgrind. I don't have any
> strong feelings on this though.

Thanks for your review.
The change is for the case that param_value_size > sizeof(device->FIELD).
For example:
  cl_ulong max_alloc_size;
  clGetDeviceInfo(device, CL_DEVICE_MAX_MEM_ALLOC_SIZE, sizeof(max_alloc_size), &max_alloc_size, NULL);

param_value_size is 8, if beignet's device->max_alloc_size is size_t, sizeof(device->max_alloc_size) is 4 in i386 systems.
Because max_alloc_size hasn't been initialized, is garbage, and memcpy(param_value, &device->FIELD, sizeof device->FIELD); 
only copy the low 4 bytes, the high 4 bytes is still garbage.
For example max_alloc_size is 0xdeaddeaddeaddead before call clGetDeviceInfo, and device-> max_alloc_size is 0x40000000,
After clGetDeviceInfo, max_alloc_size's value is 0xdeaddead40000000.

So add memset(param_value, 0, param_value_size) to clear param_value, the param_value's size is param_value_size, I think it is safe.

What do you think about?

> 
> The rest of the patch looks good to me.
> 
>    Simon
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/beignet