[PATCH v2] staging: vc04_services: rework ioctl code path

Michael Zoran mzoran at crowfest.net
Tue Nov 15 21:27:14 UTC 2016


On Tue, 2016-11-15 at 10:55 +0100, Greg KH wrote:
> On Thu, Nov 10, 2016 at 10:15:31PM -0800, Michael Zoran wrote:
> > VCHIQ/vc04_services has a userland device interface
> > that includes ioctls. The current ioctl implementation
> > is a single monolithic function over 1,000+ lines
> > that handles 17 different ioctls through a complex
> > maze of switch and if statements.
> > 
> > The change reimplements that code path by breaking
> > up the code into smaller, easier to maintain functions
> > and uses a dispatch table to invoke the correct function.
> > 
> > Testing:
> > 
> > 1. vchiq_test -f 10 and vchiq_test -p 1 were run from a native
> > 64-bit OS(debian sid).
> > 
> > 2. vchiq_test -f 10 and vchiq_test -p 1 where run from a 32-bit
> > chroot install from the same OS.
> > 
> > Both test cases pass.
> > 
> > This is V2 of this patch.  Changes include:
> > 
> > 1. More code has been moved to the dispatch routine.
> > The dispatch routine is now responsible for copying the top-level
> > data into and out of kernel space by using the data encoded in
> > the ioctl command number.
> > 
> > 2. The number of parameters have been reduced for the handling
> > functions by giving a different prototype to ioctls that pass
> > no arguments.
> > 
> > 3. Macros in linux/compat.h are now used for compatibility data
> > structures.
> > 
> > Signed-off-by: Michael Zoran <mzoran at crowfest.net>
> > ---
> >  .../vc04_services/interface/vchiq_arm/vchiq_arm.c  | 1733
> > +++++++++++++-------
> >  .../interface/vchiq_arm/vchiq_ioctl.h              |   96 ++
> >  2 files changed, 1200 insertions(+), 629 deletions(-)
> 
> This is a rough patch to review, can you break this up into a patch
> series that moves each ioctl to a separate function as needed?
> 
> thanks,
> 
> greg k-h

I'll see what I can do, but since this is getting closer to the
holidays I'm planing to spend less time on this. Perhaps someone else
may be interested in cleaning up the ioctl path?

Since the old code uses obsolete kernel features, it's going to take
some work to fix without adding any checkpatch.pl warnings.


More information about the devel mailing list