[PATCH 2/9] VC04_SERVICES: Add top level compat ioctl handler
Michael Zoran
mzoran at crowfest.net
Thu Jan 19 00:31:15 PST 2017
On Thu, 2017-01-19 at 11:00 +0300, Dan Carpenter wrote:
> On Wed, Jan 18, 2017 at 11:37:55PM -0800, Michael Zoran wrote:
> > This whole driver is a chicken and egg problem. The existing code
> > is
> > so hard to read and maintain, that it's hard to improve it in a
> > incremental way. Yet, trowing large sections out the door is too
> > hard
> > to get seriously reviewed as well...
> >
> > I would like to think that what I've submitted is an improvement on
> > the
> > existing stuff. I'm sorry you feel that no changes are possible
> > unless
> > all the issues are fixed at once.
> >
>
> This isn't fixing existing code, it's adding new code.
>
> So this morning, because you're asking me to reconsider this code, I
> looked at it again and the function that I'm complaining about seems
> totally useless. It doesn't do *anything*! I can't figure out what
> it's for. It also seems totally unused. I told myself, that I must
> be
> missing something critical hidden in macro magic, so I applied it and
> compiled it and it gives me these warnings:
>
> In file included from
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:52:0:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: In
> function ‘vchiq_ioctl_compat_internal’:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.h:185:38
> : warning: unused variable ‘debug_ptr’ [-Wunused-variable]
> #define DEBUG_INITIALISE(local) int *debug_ptr = (local)->debug;
> ^
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1222:2:
> note: in expansion of macro ‘DEBUG_INITIALISE’
> DEBUG_INITIALISE(g_state.local)
> ^
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c: At top
> level:
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:1212:1:
> warning: ‘vchiq_ioctl_compat_internal’ defined but not used [-
> Wunused-function]
> vchiq_ioctl_compat_internal(
> ^
> LD [M] drivers/staging/vc04_services/vchiq.o
>
> I shouldn't have to review code like this when the compiler already
> tells you it's unacceptable.
>
> regards,
> dan carpenter
>
Thanks for taking a second look.
Yes, it doesn't do anything. It's meant to be a placeholder for
adding other ioctls. Patch #3 begins adding calls to this function and
after adding #3 it should make more sense.
Perhaps I should have combined #2 and #3 into one patch.
More information about the linux-rpi-kernel
mailing list