linux-kernel - Re: [patch] video: build fix for drivers/media/video/mt9v022.c

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0805032004130.5182@axis700.grange>
Date:	Sat, 3 May 2008 20:15:20 +0200 (CEST)
From:	Guennadi Liakhovetski <g.liakhovetski@....de>
To:	Ingo Molnar <mingo@...e.hu>
cc:	linux-kernel@...r.kernel.org,
	Mauro Carvalho Chehab <mchehab@...radead.org>,
	Jean Delvare <khali@...ux-fr.org>
Subject: Re: [patch] video: build fix for drivers/media/video/mt9v022.c

On Sat, 3 May 2008, Ingo Molnar wrote:

> * Guennadi Liakhovetski <g.liakhovetski@....de> wrote:
> 
> > > with this config:
> > > 
> > >    http://redhat.com/~mingo/misc/config-Sat_May__3_16_08_39_CEST_2008.bad
> > > 
> > > the bug was that the driver uses GPIO functionality but only 
> > > includes the GPIO interface definitions for the 
> > > CONFIG_MT9M001_PCA9536_SWITCH case, which was not set in this 
> > > config.
> > 
> > Ok, once again a good catch and a wrong fix, sorry :-) The bug is that 
> > not CONFIG_MT9M001_PCA9536_SWITCH but CONFIG_MT9V022_PCA9536_SWITCH 
> > shall be checked for, of course. Copy-paste:-( I'll prepare a correct 
> > patch and submit it. As for the "cleanup" side - don't know. Would it 
> > be better to unconditionally include it? It won't hurt of course, 
> > looks better, but is unneeded when the GPIO is not used. And, although 
> > grep reports most drivers including asm/gpio.h, including linux/gpio.h 
> > seems indeed better.
> 
> No. If you look at the core kernel you wont see it sprinkled with things 
> like:
> 
>  #ifdef CONFIG_LOCKDEP
>  #include <linux/lockdep.h>
>  #endif
> 
> why? Because we have put all such things into the include file 
> themselves and do not want to sprinkle .c files with ugly #ifdefs. This 
> is a basic cleanliness issue.

Ok, right, sorry, the patch was indeed correct, just the reason for the 
compilation breakage was not quite what you suggested. So, I'll apply your 
putch, but please fix mt9m001.c in the same patch too - it also includes 
gpio.h conditionally. Although, it does test the correct macro.

Actually, this CONFIG option might disappear completely in the future, if 
we decide to switch to a module parameter instead. So that one kernel can 
be used on systems with and without the GPIO.

Thanks
Guennadi
---
Guennadi Liakhovetski
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/