potential parse error in ifdef

PATCH In review Yoann Padioleau 6 participants 8 messages May 21, 2007 – May 23, 2007

Subsystems (8)

Network drivers SCSI Sound Kernel janitors +4 more subsystems

lore

Share & tools

lore

Yoann Padioleau

author

2007-05-21 16:39 UTC #0 lore raw

PATCH 1/1 potential parse error in ifdef

Hi,

I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.

Signed-off-by Yoann Padioleau <[email protected]>

---

 drivers/char/watchdog/ixp2000_wdt.c |    2 +-
 drivers/mtd/devices/pmc551.c        |    2 +-
 drivers/mtd/nand/autcpu12.c         |    2 +-
 drivers/mtd/nand/ppchameleonevb.c   |    2 +-
 drivers/net/amd8111e.c              |    2 +-
 drivers/net/skfp/smt.c              |    2 +-
 drivers/scsi/aic7xxx/aic79xx_core.c |    2 +-
 sound/arm/sa11xx-uda1341.c          |    2 +-
 8 files changed, 8 insertions(+), 8 deletions(-)

drivers/char/watchdog/ixp2000_wdt.c +1 -1

index fd955db..dc7548d 100644
@@ -205,7 +205,7 @@ static void __exit ixp2000_wdt_exit(void
 module_init(ixp2000_wdt_init);
 module_exit(ixp2000_wdt_exit);
 
-MODULE_AUTHOR("Deepak Saxena <[email protected]">);
+MODULE_AUTHOR("Deepak Saxena <[email protected]>");
 MODULE_DESCRIPTION("IXP2000 Network Processor Watchdog");
 
 module_param(heartbeat, int, 0);

drivers/mtd/devices/pmc551.c +1 -1

index a4873ab..e8f686f 100644
@@ -650,7 +650,7 @@ MODULE_DESCRIPTION(PMC551_VERSION);
  */
 static int msize = 0;
 #if defined(CONFIG_MTD_PMC551_APERTURE_SIZE)
-static int asize = CONFIG_MTD_PMC551_APERTURE_SIZE
+static int asize = CONFIG_MTD_PMC551_APERTURE_SIZE;
 #else
 static int asize = 0;
 #endif

drivers/mtd/nand/autcpu12.c +1 -1

index fe94ae9..e3744eb 100644
@@ -101,7 +101,7 @@ static void autcpu12_hwcontrol(struct mt
 	struct nand_chip *chip = mtd->priv;
 
 	if (ctrl & NAND_CTRL_CHANGE) {
-		void __iomem *addr
+		void __iomem *addr;
 		unsigned char bits;
 
 		addr = CS89712_VIRT_BASE + AUTCPU12_SMC_PORT_OFFSET;

drivers/mtd/nand/ppchameleonevb.c +1 -1

index eb7d4d4..38fe9e9 100644
@@ -81,7 +81,7 @@ #ifdef CONFIG_MTD_PARTITIONS
  */
 static struct mtd_partition partition_info_hi[] = {
       { .name = "PPChameleon HI Nand Flash",
-	offset = 0,
+	.offset = 0,
 	.size = 128 * 1024 * 1024
       }
 };

drivers/net/amd8111e.c +1 -1

index 675fe91..84b8164 100644
@@ -155,7 +155,7 @@ This function will write into PHY regist
 */
 static int amd8111e_write_phy(struct amd8111e_priv* lp,int phy_id, int reg, u32 val)
 {
-	unsigned int repeat = REPEAT_CNT
+	unsigned int repeat = REPEAT_CNT;
 	void __iomem *mmio = lp->mmio;
 	unsigned int reg_val;

drivers/net/skfp/smt.c +1 -1

index fe84780..75afc1f 100644
@@ -1748,7 +1748,7 @@ char *addr_to_string(struct fddi_addr *a
 #endif
 
 #ifdef	AM29K
-smt_ifconfig(int argc, char *argv[])
+int smt_ifconfig(int argc, char *argv[])
 {
 	if (argc >= 2 && !strcmp(argv[0],"opt_bypass") &&
 	    !strcmp(argv[1],"yes")) {

drivers/scsi/aic7xxx/aic79xx_core.c +1 -1

index 9ddc6e4..05f692b 100644
@@ -5180,7 +5180,7 @@ #ifdef AHD_TARGET_MODE
 			cur_lun = lun;
 			max_lun = lun;
 		}
-		for (cur_lun <= max_lun; cur_lun++) {
+		for (;cur_lun <= max_lun; cur_lun++) {
 			struct ahd_tmode_lstate* lstate;
 
 			lstate = tstate->enabled_luns[cur_lun];

sound/arm/sa11xx-uda1341.c +1 -1

index c7e1b26..e7ed868 100644
@@ -987,7 +987,7 @@ static int __init sa11xx_uda1341_init(vo
 		if (platform_get_drvdata(device))
 			return 0;
 		platform_device_unregister(device);
-		err = -ENODEV
+		err = -ENODEV;
 	} else
 		err = PTR_ERR(device);
 	platform_driver_unregister(&sa11xx_uda1341_driver);

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Matthew Wilcox

Acked-by on v1 1/1

2007-05-21 17:01 UTC #1 lore raw

Quoted 6 lines · Yoann Padioleau

I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.

These all look good to me.

Acked-by Matthew Wilcox <[email protected]>

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Yoann Padioleau

author

2007-05-22 08:21 UTC #2 lore raw

In reply to Yoann Padioleau

Quoted 9 lines · Matthew Wilcox

Quoted 6 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.
These all look good to me.

Thanks.

Quoted 2 lines

Acked-by Matthew Wilcox <[email protected]>

Should I CC my patch to LKML ?

Quoted 4 lines

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Håkon Løvdal

2007-05-22 23:10 UTC #3 lore raw

In reply to Yoann Padioleau

Quoted 9 lines · Matthew Wilcox

Quoted 6 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.
These all look good to me.

But one question. His tool has found some code segments that obviously never have been compiled and most likely are not included in the kernel in any way (because otherwise I would assume that allyesconfig would have caught them).

Dead, unused code that theoretically would compile is probably better than dead, unused code that would not compile, but if the code in question is indeed dead and unused I would assume that the best option would be to remove it?

BR Håkon Løvdal

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Cripps

2007-05-22 23:12 UTC #4 lore raw

In reply to Yoann Padioleau

best way to answer that: try removing the code and testing with "make allyesconfig" et al ... The big thing about removing said segments of code is: where else are bits of code that rely on this hiding throughout the kernel? Once everything is found, if all of the code does nothing, haul 'er outta there, else, try and minimise the code that *is* in there so that the remaining kernel code it "prettier." -Aaron Cripps

P.S. that IS a good question ...once we have a working patch, that is tested and does what it's supposed to do, *do* we send it upstream to the LKML, or does someone work on making a digest of all the patches sent to [KJ] and send that upstream?

Quoted 22 lines · Håkon Løvdal

Quoted 9 lines · Matthew Wilcox
Quoted 6 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.
These all look good to me.
But one question. His tool has found some code segments that obviously never have been compiled and most likely are not included in the kernel in any way (because otherwise I would assume that allyesconfig would have caught them).
Dead, unused code that theoretically would compile is probably better than dead, unused code that would not compile, but if the code in question is indeed dead and unused I would assume that the best option would be to remove it?

Quoted 7 lines

BR Håkon Løvdal
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

walter harms

2007-05-23 07:32 UTC #5 lore raw

In reply to Yoann Padioleau

hi Håkon,
you are right in some way but look in these (theoretical) example:

#ifdef LINUX
	donea()
#else
	doneb()

#endif

doneb() will never be used on Linux but it may be useful for a driver programmer to run that thing on an other system.

IMHO: add a note like "please explain why we need that dead code or we remove it", send it to the maintainer and let him remove either the code or add an explanation.

re,
wh

Quoted 23 lines · Håkon Løvdal

Quoted 10 lines · Matthew Wilcox
Quoted 2 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code,
including
Quoted 4 lines
code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.
These all look good to me.
But one question. His tool has found some code segments that obviously never have been compiled and most likely are not included in the kernel in any way (because otherwise I would assume that allyesconfig would have caught them).
Dead, unused code that theoretically would compile is probably better than dead, unused code that would not compile, but if the code in question is indeed dead and unused I would assume that the best option would be to remove it?

Quoted 9 lines

BR Håkon Løvdal
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Yoann Padioleau

author

2007-05-23 07:46 UTC #6 lore raw

In reply to Yoann Padioleau

Quoted 9 lines · Håkon Løvdal

Quoted 8 lines · Matthew Wilcox
Quoted 6 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.

Quoted 4 lines

Quoted 1 line
These all look good to me.
But one question. His tool has found some code segments that obviously never have been compiled

I think so

Quoted 3 lines

and most likely are
not included in the kernel in any way (because otherwise I would assume that allyesconfig would have caught them).

Allyesconfig does not compile everything. It does not compile for example source code for other architectures, nor drivers made for other architectures and it does not try all the ifdef combinaisons.

Quoted 4 lines

Dead, unused code that theoretically would compile is probably better than dead, unused code that would not compile,

Except my patch can only improve the situation.

Quoted 10 lines

but if the code in question is indeed dead and unused I would assume that the best option would be to remove it?
BR Håkon Løvdal
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors

Robert P. J. Day

2007-05-23 08:18 UTC #7 lore raw

In reply to Yoann Padioleau

Quoted 18 lines · Yoann Padioleau

Quoted 9 lines · Håkon Løvdal
Quoted 8 lines · Matthew Wilcox
Quoted 6 lines · Yoann Padioleau
I have made a tool to parse the kernel that does not pre-process the source. That means that my parser tries to parse all the code, including code in the #else branch or code that is not often compiled because the driver is not very used (or not used at all). So, my parser sometimes reports parse error not originally detected by gcc. Here is my (first) patch.
Quoted 4 lines
Quoted 1 line
These all look good to me.
But one question. His tool has found some code segments that obviously never have been compiled
I think so

Quoted 7 lines

Quoted 3 lines
and most likely are
not included in the kernel in any way (because otherwise I would assume that allyesconfig would have caught them).
Allyesconfig does not compile everything. It does not compile for example source code for other architectures, nor drivers made for other architectures and it does not try all the ifdef combinaisons.

i wrote a script that did something sort of like this a while back. after a build with "allyesconfig", the script scanned for all source files for which a corresponding object file wasn't generated.

needless to say, there were *lots* of those since some source files represent a choice among several possibilities, while there were entire subdirectories that weren't compiled for that very reason above -- they were for a different arch.

but digging into the output, i did find a few cases where some source files were simply not part of the build process anymore, and submitted patches to have them removed.

rday

Signature · 7 lines

========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://fsdev.net/wiki/index.php?title=Main_Page
========================================================================

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/kernel-janitors