On 29 February 2016 at 16:16, Tvrtko Ursulin
<tvrtko.ursulin@xxxxxxxxxxxxxxx> wrote:
>
> On 29/02/16 11:13, Dave Gordon wrote:
>>
>> After the recent addition of drm_malloc_gfp(), it was noticed that
>> some callers of these functions has swapped the parameters in the
>> call - it's supposed to be 'number of members' and 'sizeof(element)',
>> but a few callers had got the size first and the count second. This
>> isn't otherwise detected because they're both type 'size_t', and
>> the implementation at present just multiplies them anyway, so the
>> result is still right. But some future implementation might treat
>> them differently (e.g. allowing 0 elements but not zero size), so
>> let's add some compile-time checks and complain if the second (size)
>> parameter isn't a sizeof() expression, or at least a compile-time
>> constant.
>>
>> This patch also fixes those callers where the order was wrong.
>>
>> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
>> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>Cc: dri-
>> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
>> ---
>> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +-
>> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 8 ++++----
>> include/drm/drm_mem_util.h | 30
>> +++++++++++++++++++++++++---
>> 3 files changed, 32 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> index 1aba01a..9ae4a71 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>> @@ -340,7 +340,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev,
>> void *data,
>> */
>> bos = drm_malloc_ab(args->nr_bos, sizeof(*bos));
>> relocs = drm_malloc_ab(args->nr_relocs, sizeof(*relocs));
>> - stream = drm_malloc_ab(1, args->stream_size);
>> + stream = drm_malloc_ab(args->stream_size, sizeof(*stream));
>
>
> I was surprised sizeof(void) == 1. On further research that seems to be an
> GCC extension.
>
Afaict pointer arithmetic (-Wpointer-arith) has/is been used in the
kernel extensively. In userspace we try to avoid it libdrm and mesa,
there might be a few cases where it's still around. No too sure about
libva{,-intel}.
> I am not sure how active projects to compile the kernel with for example ICC
> are, just remember some talks about it in the past. Maybe it is even
> possible? In that case it would be better to just leave "1" there to not
> rely on GCC extensions.
>
While I'm all for the idea, I doubt we'll get there any time soon. My
last suggestion (do not use zero sized arrays) went down in flames :-(
Regards,
Emil
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
