On Wed, Nov 6, 2019 at 5:52 AM Andrew F. Davis <afd@xxxxxx> wrote:
>
> On 11/5/19 11:22 PM, John Stultz wrote:
> > +unsigned int dma_heap_ioctl_cmds[] = {
> > + DMA_HEAP_IOC_ALLOC,
> > +};
> > +
> > +static long dma_heap_ioctl(struct file *file, unsigned int ucmd,
> > + unsigned long arg)
> > +{
> > + char stack_kdata[128];
> > + char *kdata = stack_kdata;
> > + unsigned int kcmd;
> > + unsigned int in_size, out_size, drv_size, ksize;
> > + int nr = _IOC_NR(ucmd);
> > + int ret = 0;
> > +
> > + if (nr >= ARRAY_SIZE(dma_heap_ioctl_cmds))
> > + return -EINVAL;
> > +
> > + /* Get the kernel ioctl cmd that matches */
> > + kcmd = dma_heap_ioctl_cmds[nr];
>
>
> Why do we need this indirection here and all the complexity below? I
> know DRM ioctl does something like this but it has a massive table,
> legacy ioctls, driver defined ioctls, etc..
>
> I don't expect we will ever need complex handling like this, could we
> switch back to the more simple handler from v13?
I agree it does add complexity, but I'm not sure I see how to avoid
some of this. The logic trying to handle that the user may pass a cmd
that has the same _IOC_NR() as DMA_HEAP_IOC_ALLOC but not the same
size. So the simple "switch(cmd) { case DMA_HEAP_IOC_ALLOC:" we had
before won't work (as the cmd will be a different value).
Thus why I thought the cleanest approach would be to use the
dma_heap_ioctl_cmds array to convert from whatever the user cmd is to
the matching kernel cmd value.
Do you have an alternative suggestion that I'm overlooking?
thanks
-john
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
