Skip to contentSkip to conversation

Fixups for issues when running 'make randconfig'

PATCH Applied Lee Jones 9 participants 47 messages Nov 3, 2012 – Nov 9, 2012

Subsystems (13)
lore
Share & tools
lore

Lee Jones

author
#0 lore raw

cover Fixups for issues when running 'make randconfig'

This patch-set is the result of fixing some problems encountered when building the kernel with randconfig and a when running a script I wrote to identify unused structures.

 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 arch/arm/mach-integrator/cpu.c           |   12 ------------
 arch/x86/kernel/quirks.c                 |    4 ++--
 drivers/isdn/i4l/isdn_common.c           |    1 -
 drivers/virtio/virtio_mmio.c             |    2 +-
 fs/quota/quota.c                         |    4 ++++
 kernel/sched/fair.c                      |    4 ++++
 net/bridge/br_private.h                  |    8 ++++----
 sound/soc/codecs/Kconfig                 |    2 +-
 9 files changed, 16 insertions(+), 41 deletions(-)
author
#1 lore raw

PATCH 1/9 ARM: integrator: Remove unused icst_params lclk_params structure

This was introduced way back before 2005 and has remained unused for over 7 years. Let's remove it.

Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-integrator/cpu.c |   12 ------------
 1 file changed, 12 deletions(-)
arch/arm/mach-integrator/cpu.c +0 -12
index 590c192..4e0a689 100644
@@ -30,18 +30,6 @@ static struct cpufreq_driver integrator_driver;
 #define CM_STAT __io_address(INTEGRATOR_HDR_STAT)
 #define CM_LOCK __io_address(INTEGRATOR_HDR_LOCK)
 
-static const struct icst_params lclk_params = {
-	.ref		= 24000000,
-	.vco_max	= ICST525_VCO_MAX_5V,
-	.vco_min	= ICST525_VCO_MIN,
-	.vd_min		= 8,
-	.vd_max		= 132,
-	.rd_min		= 24,
-	.rd_max		= 24,
-	.s2div		= icst525_s2div,
-	.idx2s		= icst525_idx2s,
-};
-
 static const struct icst_params cclk_params = {
 	.ref		= 24000000,
 	.vco_max	= ICST525_VCO_MAX_5V,
Signature · 1 line
1.7.9.5
author
#2 lore raw

PATCH 2/9 ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

In reply to Lee Jones

This the at91sam9g45_isi_device structure and its associated resources were added in 2008 and have been unused ever since. Let's remove them.

Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)
arch/arm/mach-at91/at91sam9263_devices.c +0 -20
index cb85da2..0562a9d 100644
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
Signature · 1 line
1.7.9.5
#3 lore raw

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

Quoted 2 lines · Lee Jones

This the at91sam9g45_isi_device structure and its associated resources were added in 2008 and have been unused ever since. Let's remove them.

next time you need to Cc the matinaers

NACK

Best Regards,
J.

Quoted 47 lines
Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)
arch/arm/mach-at91/at91sam9263_devices.c +0 -20
index cb85da2..0562a9d 100644
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
Signature · 7 lines
1.7.9.5


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
author
#4 lore raw

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

Quoted 4 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 2 lines · Lee Jones

This the at91sam9g45_isi_device structure and its associated resources were added in 2008 and have been unused ever since. Let's remove them.

next time you need to Cc the matinaers

I did:

$ git show <sha1> | scripts/get_maintainer.pl
  Russell King <[email protected]> (maintainer:ARM PORT)
  linux-arm-kernel at lists.infradead.org (moderated list:ARM SUB-ARCHITECT...)
  linux-kernel at vger.kernel.org (open list)
Quoted 1 line

NACK

Any particular reason?

Quoted 46 lines
Quoted 46 lines
Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)
arch/arm/mach-at91/at91sam9263_devices.c +0 -20
index cb85da2..0562a9d 100644
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
Signature · 7 lines
1.7.9.5


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#5 lore raw

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

In reply to Lee Jones

Hi, Lee Jones

Quoted 2 lines · Lee Jones

This the at91sam9g45_isi_device structure and its associated resources were added in 2008 and have been unused ever since. Let's remove them.

I'm the maintainer of the Atmel ISI driver. Currently the ISI still not work on at91sam9263 board.
But this task is in my plan. So keep those code and I will enable ISI support for 9263 in the future. Thanks.

Best Regards,
Josh Wu

Quoted 38 lines
Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
  arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
  1 file changed, 20 deletions(-)
arch/arm/mach-at91/at91sam9263_devices.c +0 -20
index cb85da2..0562a9d 100644
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 
  #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
 
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
  void __init at91_add_device_isi(struct isi_platform_data *data,
  		bool use_pck_as_mck)
  {
author
#6 lore raw

Re: [PATCH 2/9] ARM: at91: Remove unused struct at91sam9g45_isi_device and its resources

Quoted 7 lines
Quoted 2 lines

This the at91sam9g45_isi_device structure and its associated resources were added in 2008 and have been unused ever since. Let's remove them.

I'm the maintainer of the Atmel ISI driver. Currently the ISI still not work on at91sam9263 board.
But this task is in my plan. So keep those code and I will enable ISI support for 9263 in the future. Thanks.

No problem at all. Thanks for getting back to me Josh.

Kind regards,
Lee

Quoted 37 lines
Quoted 36 lines
Cc Russell King <[email protected]>
Cc linux-arm-kernel at lists.infradead.org
---
 arch/arm/mach-at91/at91sam9263_devices.c |   20 --------------------
 1 file changed, 20 deletions(-)
arch/arm/mach-at91/at91sam9263_devices.c +0 -20
index cb85da2..0562a9d 100644
@@ -901,26 +901,6 @@ void __init at91_add_device_lcdc(struct atmel_lcdfb_info *data) {}
 #if defined(CONFIG_VIDEO_AT91_ISI) || defined(CONFIG_VIDEO_AT91_ISI_MODULE)
-struct resource isi_resources[] = {
-	[0] = {
-		.start	= AT91SAM9263_BASE_ISI,
-		.end	= AT91SAM9263_BASE_ISI + SZ_16K - 1,
-		.flags	= IORESOURCE_MEM,
-	},
-	[1] = {
-		.start	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.end	= NR_IRQS_LEGACY + AT91SAM9263_ID_ISI,
-		.flags	= IORESOURCE_IRQ,
-	},
-};
-
-static struct platform_device at91sam9263_isi_device = {
-	.name		= "at91_isi",
-	.id		= -1,
-	.resource	= isi_resources,
-	.num_resources	= ARRAY_SIZE(isi_resources),
-};
-
 void __init at91_add_device_isi(struct isi_platform_data *data,
 		bool use_pck_as_mck)
 {
Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#7 lore raw

PATCH 3/9 x86: Suppress build warnings in quirks

In reply to Lee Jones

arch/x86/kernel/quirks.c: In function ‘ati_force_enable_hpet’:
arch/x86/kernel/quirks.c:364:4: warning: ‘d’ may be used uninitialised in this function [-Wuninitialized] arch/x86/kernel/quirks.c:357:6: note: ‘d’ was declared here arch/x86/kernel/quirks.c:407:21: warning: ‘val’ may be used uninitialised in this function [-Wuninitialized]

---
 arch/x86/kernel/quirks.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/x86/kernel/quirks.c +2 -2
index 1b27de5..d280b70 100644
@@ -354,7 +354,7 @@ static void ati_force_hpet_resume(void)
 
 static u32 ati_ixp4x0_rev(struct pci_dev *dev)
 {
-	u32 d;
+	u32 d = 0;
 	u8  b;
 
 	pci_read_config_byte(dev, 0xac, &b);
@@ -371,7 +371,7 @@ static u32 ati_ixp4x0_rev(struct pci_dev *dev)
 
 static void ati_force_enable_hpet(struct pci_dev *dev)
 {
-	u32 d, val;
+	u32 d, val = 0;
 	u8  b;
 
 	if (hpet_address || force_hpet_address)
Signature · 1 line
1.7.9.5
author
#8 lore raw

PATCH 4/9 sched: Don't define unthrottle_offline_cfs_rqs when !CONFIG_SMP

In reply to Lee Jones

Since unthrottle_offline_cfs_rqs is only ever invoked when CONFIG_SMP let's pre-processor it out when SMP is not configured. This change suppresses the build error below when !CONFIG_SMP.

kernel/sched/fair.c:2055:13: warning: ‘unthrottle_offline_cfs_rqs’ defined but not used [-Wunused-function]

Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 kernel/sched/fair.c |    4 ++++
 1 file changed, 4 insertions(+)
kernel/sched/fair.c +4 -0
index 6b800a1..c0aab13 100644
@@ -2052,6 +2052,7 @@ static void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b)
 	hrtimer_cancel(&cfs_b->slack_timer);
 }
 
+#if defined CONFIG_SMP
 static void unthrottle_offline_cfs_rqs(struct rq *rq)
 {
 	struct cfs_rq *cfs_rq;
@@ -2071,6 +2072,7 @@ static void unthrottle_offline_cfs_rqs(struct rq *rq)
 			unthrottle_cfs_rq(cfs_rq);
 	}
 }
+#endif /* CONFIG_SMP */
 
 #else /* CONFIG_CFS_BANDWIDTH */
 static __always_inline
@@ -2106,7 +2108,9 @@ static inline struct cfs_bandwidth *tg_cfs_bandwidth(struct task_group *tg)
 	return NULL;
 }
 static inline void destroy_cfs_bandwidth(struct cfs_bandwidth *cfs_b) {}
+#if defined CONFIG_SMP
 static inline void unthrottle_offline_cfs_rqs(struct rq *rq) {}
+#endif /* CONFIG_SMP */
 
 #endif /* CONFIG_CFS_BANDWIDTH */
 
Signature · 1 line
1.7.9.5
author
#9 lore raw

PATCH 5/9 ASoC: Only compile adav80x codec when a bus is defined

In reply to Lee Jones

If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support for the adav80x codec as it relies on a bus to operate. This patch fixes the build warnings below when SPI and I2C are not specified in the configuration.

sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function] sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]

---
 sound/soc/codecs/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/Kconfig +1 -1
index b92759a..0d8135a 100644
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if (I2C || I2C_MODULE || SPI_MASTER)
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
Signature · 6 lines
1.7.9.5

_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
#10 lore raw

Re: [alsa-devel] [PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

Quoted 20 lines · Lee Jones

If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support for the adav80x codec as it relies on a bus to operate. This patch fixes the build warnings below when SPI and I2C are not specified in the configuration.

sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function] sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]

---
 sound/soc/codecs/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/soc/codecs/Kconfig +0 -0
index b92759a..0d8135a 100644

sound/soc/codecs/Kconfig config SND_SOC_ALL_CODECS · line 22

Quoted 5 lines
sound/soc/codecs/Kconfig +1 -1
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if (I2C || I2C_MODULE || SPI_MASTER)

That should be 'if SND_SOC_I2C_AND_SPI', but otherwise agreed.

Quoted 3 lines
sound/soc/codecs/Kconfig +0 -0
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
author
#11 lore raw

Re: [alsa-devel] [PATCH 5/9] ASoC: Only compile adav80x codec when a bus is defined

Thanks for reviewing Lars. Here's the result:

commit c4f60d00d04abfb730a54a97204873dafc6473ae

Author: Lee Jones <[email protected]>
Date:   Sat Nov 3 22:54:23 2012 +0100

ASoC: Only compile adav80x codec when a bus is defined

If !CONFIG_SPI and !CONFIG_I2C* then there is no point adding support for the adav80x codec as it relies on a bus to operate. This patch fixes the build warnings below when SPI and I2C are not specified in the configuration.

sound/soc/codecs/adav80x.c:842:22: warning: ‘adav80x_bus_probe’ defined but not used [-Wunused-function] sound/soc/codecs/adav80x.c:863:22: warning: ‘adav80x_bus_remove’ defined but not used [-Wunused-function]

sound/soc/codecs/Kconfig +1 -1
index b92759a..3ae3a4b 100644
@@ -19,7 +19,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_AD1980 if SND_SOC_AC97_BUS
 	select SND_SOC_AD73311
 	select SND_SOC_ADAU1373 if I2C
-	select SND_SOC_ADAV80X
+	select SND_SOC_ADAV80X if SND_SOC_I2C_AND_SPI
 	select SND_SOC_ADS117X
 	select SND_SOC_AK4104 if SPI_MASTER
 	select SND_SOC_AK4535 if I2C
author
#12 lore raw

PATCH 6/9 virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

In reply to Lee Jones

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/virtio/virtio_mmio.c +1 -1
index 6b1b7e1..077e9ca 100644
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);
 
 	if (processed < 2 || processed > 3 || str[consumed])
Signature · 6 lines
1.7.9.5

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
author
#13 lore raw

PATCH 6/9 virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

In reply to Lee Jones

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/virtio/virtio_mmio.c +1 -1
index 6b1b7e1..077e9ca 100644
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);
 
 	if (processed < 2 || processed > 3 || str[consumed])
Signature · 1 line
1.7.9.5
#14 lore raw

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

Quoted 15 lines · Lee Jones

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/virtio/virtio_mmio.c +0 -0
index 6b1b7e1..077e9ca 100644

drivers/virtio/virtio_mmio.c static int vm_cmdline_set(const char *device, · line 535

Quoted 6 lines
drivers/virtio/virtio_mmio.c +1 -1
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);

This suppresses a valid warning, leaving our code no less wrong than before. But now it's silently wrong!

Do you want to try again?

Cheers,
Rusty.
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

author
#15 lore raw

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

Quoted 26 lines · Rusty Russell
Quoted 15 lines · Lee Jones

drivers/virtio/virtio_mmio.c: In function ‘vm_cmdline_set’:
drivers/virtio/virtio_mmio.c:535:4: warning: format ‘%u’ expects argument of type ‘unsigned int *’, but argument 4 has type ‘resource_size_t *’ [-Wformat]

---
 drivers/virtio/virtio_mmio.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/virtio/virtio_mmio.c +0 -0
index 6b1b7e1..077e9ca 100644

drivers/virtio/virtio_mmio.c static int vm_cmdline_set(const char *device, · line 535

Quoted 6 lines
drivers/virtio/virtio_mmio.c +1 -1
@@ -531,7 +531,7 @@ static int vm_cmdline_set(const char *device,
 	resources[0].end = memparse(device, &str) - 1;
 
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);

This suppresses a valid warning, leaving our code no less wrong than before. But now it's silently wrong!

Quoted 1 line

Do you want to try again?

For sure I'll try again.

How does s/%u/%p/ suit?

Signature · 8 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
#16 lore raw

Re: [PATCH 6/9] virtio_mmio: Cast &resources[1].start to ‘unsigned int *’ to rid compiler warning

Quoted 8 lines · Lee Jones
Quoted 8 lines

drivers/virtio/virtio_mmio.c static int vm_cmdline_set(const char *device, · line 535

Quoted 4 lines
drivers/virtio/virtio_mmio.c +1 -1
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, (unsigned int *)&resources[1].start, &consumed,
 			&vm_cmdline_id, &consumed);

This suppresses a valid warning, leaving our code no less wrong than before. But now it's silently wrong!

Quoted 5 lines
Quoted 1 line

Do you want to try again?

For sure I'll try again.

How does s/%u/%p/ suit?

sscanf doesn't do "%p"... The only reasonable way of fixing this is a intermediate local variable - will post a patch in a second.

Thanks for spotting this!

Paweł

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

#17 lore raw

PATCH 1/1 virtio-mmio: Fix irq parsing in command line parameter

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)
drivers/virtio/virtio_mmio.c +18 -8
index 6b1b7e1..0d08843 100644
@@ -521,25 +521,35 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Get "size" part of the command line parameter */
+	size = memparse(device, &str) - 1;
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;
 
+	/* Memory resource */
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size;
+
+	/* Interrupt resource */
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
Signature · 1 line
1.7.10.4
author
#18 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

Quoted 4 lines · Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Quoted 8 lines

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

Resorted to poaching now have we Pawel? ;)

Signature · 8 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
#19 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

Quoted 9 lines · Lee Jones
Quoted 7 lines · Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

Quoted 7 lines
Quoted 5 lines
---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

Resorted to poaching now have we Pawel? ;)

Sure thing - the poached eggs are my firm favourite :-P ;-)

But seriously speaking - it's an old patch (all the comments and resources creation, the IRQ stuff is a new thing :-), but never had enough motivation to push it out...

Cheers!

Paweł

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

author
#20 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

Quoted 16 lines · Pawel Moll
Quoted 15 lines · Lee Jones
Quoted 12 lines · Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)
Quoted 7 lines
Quoted 1 line

Resorted to poaching now have we Pawel? ;)

Sure thing - the poached eggs are my firm favourite :-P ;-)

But seriously speaking - it's an old patch (all the comments and resources creation, the IRQ stuff is a new thing :-), but never had enough motivation to push it out...

I believe you, many would not. ;)

Signature · 8 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
#21 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

In reply to Lee Jones
Quoted 9 lines · Lee Jones
Quoted 7 lines · Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

Quoted 7 lines
Quoted 5 lines
---
 drivers/virtio/virtio_mmio.c |   26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

Resorted to poaching now have we Pawel? ;)

I hope you were joking! Doing your work for you isn't poaching.

Your correct response was "thanks" followed by 'Tested-by:'.

Cheers,
Rusty.

author
#22 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

Quoted 3 lines
Quoted 1 line

Resorted to poaching now have we Pawel?

I hope you were joking!

Yes, of course. I thought that was clearly indicated by the jovial winking smiley. :)

I realise it wasn't obvious soley by this exchange, but Pawel and I are actually ol' friends.

Quoted 1 line

Doing your work for you isn't poaching.

This isn't related to my work, as I have no professional interest in virtio. In this particular instance I was trying to help out by fixing bugs uncovered by the use of randconfig, which I'm doing in my own time, as a hobby. I guess the use my work address clouds this fact, so I apologise for that.

Sorry for any confusion or offence caused though Rusty. :)

Kind regards,
Lee

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

#23 lore raw

Re: [PATCH] virtio-mmio: Fix irq parsing in command line parameter

Quoted 18 lines · Lee Jones
Quoted 3 lines
Quoted 1 line

Resorted to poaching now have we Pawel?

I hope you were joking!

Yes, of course. I thought that was clearly indicated by the jovial winking smiley. :)

I realise it wasn't obvious soley by this exchange, but Pawel and I are actually ol' friends.

Quoted 1 line

Doing your work for you isn't poaching.

This isn't related to my work, as I have no professional interest in virtio. In this particular instance I was trying to help out by fixing bugs uncovered by the use of randconfig, which I'm doing in my own time, as a hobby. I guess the use my work address clouds this fact, so I apologise for that.

Quoted 4 lines

Sorry for any confusion or offence caused though Rusty. :)

Kind regards,
Lee

No offence for me. I just don't want bystanders to believe that there is some etiquette other than "best patch wins".

Thanks for the clarification!
Rusty.

#24 lore raw

PATCH v2 1/1 virtio-mmio: Fix irq parsing in the command line

In reply to Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and adde extra comments to make the parameters parsing easier to follow.

---
 drivers/virtio/virtio_mmio.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Comparing to v1 I've simply "polished" the size assignment (moved -1 from one place to another) and removed two of the new comments, as they weren't really useful any more.

drivers/virtio/virtio_mmio.c +16 -8
index 6b1b7e1..e807c2a 100644
@@ -521,25 +521,33 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Consume "size" part of the command line parameter */
+	size = memparse(device, &str);
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;
 
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size - 1;
+
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
Signature · 1 line
1.7.10.4
#25 lore raw

Re: [PATCH v2] virtio-mmio: Fix irq parsing in the command line

Quoted 3 lines · Pawel Moll

On 64-bit machines resource_size_t is a 64-bit value, while sscanf() format for this argument was defined as "%u". Fixed by using an intermediate local value of a known length.

Actually, on 32-bit machines, too (eg. x86 with PAE). Otherwise we'd just change it to %lu.

drivers/virtio/virtio_mmio.c static int vm_cmdline_set(const char *device, · line 543

Quoted 13 lines
drivers/virtio/virtio_mmio.c +7 -1
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
+	/*
+	 * sscanf() processes 3 chunks if "<id>" is given, 2 if not;
+	 * also there must be no extra characters after the last
+	 * chunk, so str[consumed] should be '\0'
+	 */
 	if (processed < 2 || processed > 3 || str[consumed])
 		return -EINVAL;

I would drop the > 3 case. It's unnecessary, and you're assuming consumed doesn't add to the count, which may be true but is a documented sscanf weirdness so I don't like to rely on it.

Thanks,
Rusty.

#26 lore raw

PATCH v3 1/1 virtio-mmio: Fix irq parsing in command line parameter

When the resource_size_t is 64-bit long, the sscanf() on the virtio device command line paramter string may return wrong value because its format was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and added extra comments to make the parameters parsing easier to follow.

---
 drivers/virtio/virtio_mmio.c |   26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)
drivers/virtio/virtio_mmio.c +17 -9
index 6b1b7e1..9df4578 100644
@@ -521,25 +521,33 @@ static int vm_cmdline_set(const char *device,
 	int err;
 	struct resource resources[2] = {};
 	char *str;
-	long long int base;
+	long long int base, size;
+	unsigned int irq;
 	int processed, consumed = 0;
 	struct platform_device *pdev;
 
-	resources[0].flags = IORESOURCE_MEM;
-	resources[1].flags = IORESOURCE_IRQ;
-
-	resources[0].end = memparse(device, &str) - 1;
+	/* Consume "size" part of the command line parameter */
+	size = memparse(device, &str);
 
+	/* Get "@<base>:<irq>[:<id>]" chunks */
 	processed = sscanf(str, "@%lli:%u%n:%d%n",
-			&base, &resources[1].start, &consumed,
+			&base, &irq, &consumed,
 			&vm_cmdline_id, &consumed);
 
-	if (processed < 2 || processed > 3 || str[consumed])
+	/*
+	 * sscanf() must processes at least 2 chunks; also there
+	 * must be no extra characters after the last chunk, so
+	 * str[consumed] must be '\0'
+	 */
+	if (processed < 2 || str[consumed])
 		return -EINVAL;
 
+	resources[0].flags = IORESOURCE_MEM;
 	resources[0].start = base;
-	resources[0].end += base;
-	resources[1].end = resources[1].start;
+	resources[0].end = base + size - 1;
+
+	resources[1].flags = IORESOURCE_IRQ;
+	resources[1].start = resources[1].end = irq;
 
 	if (!vm_cmdline_parent_registered) {
 		err = device_register(&vm_cmdline_parent);
Signature · 1 line
1.7.10.4
#27 lore raw

Re: [PATCH v3] virtio-mmio: Fix irq parsing in command line parameter

Quoted 7 lines · Pawel Moll

When the resource_size_t is 64-bit long, the sscanf() on the virtio device command line paramter string may return wrong value because its format was defined as "%u". Fixed by using an intermediate local value of a known length.

Also added cleaned up the resource creation and added extra comments to make the parameters parsing easier to follow.

Applied.

Thanks,
Rusty.

author
#28 lore raw

PATCH 7/9 quota: Use the pre-processor to compile out quotactl_cmd_write when !CONFIG_BLOCK

In reply to Lee Jones

quotactl_cmd_write() is only ever invoked when BLOCK is configured. When !CONFIG_BLOCK, the build warning below is displayed. Let's fix that.

fs/quota/quota.c:311:12: warning: ‘quotactl_cmd_write’ defined but not used [-Wunused-function]

---
 fs/quota/quota.c |    4 ++++
 1 file changed, 4 insertions(+)
fs/quota/quota.c +4 -0
index af1661f..c7314f1 100644
@@ -307,6 +307,8 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 	}
 }
 
+#ifdef CONFIG_BLOCK
+
 /* Return 1 if 'cmd' will block on frozen filesystem */
 static int quotactl_cmd_write(int cmd)
 {
@@ -322,6 +324,8 @@ static int quotactl_cmd_write(int cmd)
 	return 1;
 }
 
+#endif /* CONFIG_BLOCK */
+
 /*
  * look up a superblock on which quota ops will be performed
  * - use the name of a block device to find the superblock thereon
Signature · 1 line
1.7.9.5
#29 lore raw

Re: [PATCH 7/9] quota: Use the pre-processor to compile out quotactl_cmd_write when !CONFIG_BLOCK

Quoted 4 lines · Lee Jones

quotactl_cmd_write() is only ever invoked when BLOCK is configured. When !CONFIG_BLOCK, the build warning below is displayed. Let's fix that.

fs/quota/quota.c:311:12: warning: ‘quotactl_cmd_write’ defined but not used [-Wunused-function]

Looks good. I've added the patch to my tree.

Honza

Quoted 32 lines
---
 fs/quota/quota.c |    4 ++++
 1 file changed, 4 insertions(+)
fs/quota/quota.c +4 -0
index af1661f..c7314f1 100644
@@ -307,6 +307,8 @@ static int do_quotactl(struct super_block *sb, int type, int cmd, qid_t id,
 	}
 }
 
+#ifdef CONFIG_BLOCK
+
 /* Return 1 if 'cmd' will block on frozen filesystem */
 static int quotactl_cmd_write(int cmd)
 {
@@ -322,6 +324,8 @@ static int quotactl_cmd_write(int cmd)
 	return 1;
 }
 
+#endif /* CONFIG_BLOCK */
+
 /*
  * look up a superblock on which quota ops will be performed
  * - use the name of a block device to find the superblock thereon
Signature · 1 line
1.7.9.5
Signature · 2 lines
Jan Kara <[email protected]>
SUSE Labs, CR
author
#30 lore raw

PATCH 8/9 isdn: Remove unused variable causing a compile build warning

In reply to Lee Jones

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

---
 drivers/isdn/i4l/isdn_common.c |    1 -
 1 file changed, 1 deletion(-)
drivers/isdn/i4l/isdn_common.c +0 -1
index 8c610fa..fb90d40 100644
@@ -1275,7 +1275,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 	int ret;
 	int i;
 	char __user *p;
-	char *s;
 	union iocpar {
 		char name[10];
 		char bname[22];
Signature · 1 line
1.7.9.5
#31 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 3 lines · Lee Jones

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Paul Bolle

author
#32 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 6 lines · Paul Bolle
Quoted 3 lines · Lee Jones

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

I'll fix up and resend. Will likely be tomorrow now, as it's fast approaching midnight here.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#33 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 11 lines · Lee Jones
Quoted 6 lines · Paul Bolle
Quoted 3 lines · Lee Jones

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense.

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.)

Paul Bolle

author
#34 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 13 lines · Paul Bolle
Quoted 11 lines · Lee Jones
Quoted 6 lines · Paul Bolle
Quoted 3 lines · Lee Jones

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

Quoted 8 lines

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense.

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.)

I'm in the same position as you Paul. I just noticed the warning so fixed it following the current way of doing things. Any, more substantial changes requiring greater knowledge of the subsystem would have to be done by someone else.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#35 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

From: Lee Jones <[email protected]>
Date: Sun, 4 Nov 2012 11:53:32 +0100

Quoted 20 lines · Paul Bolle
Quoted 19 lines · Lee Jones
Quoted 11 lines · Paul Bolle
Quoted 6 lines · Lee Jones
Quoted 3 lines

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense.

Quoted 8 lines
Quoted 3 lines

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.)

I'm in the same position as you Paul. I just noticed the warning so fixed it following the current way of doing things. Any, more substantial changes requiring greater knowledge of the subsystem would have to be done by someone else.

I think the most appropriate thing to do is make CONFIG_ISDN depend upon CONFIG_NETDEVICES in the Kconfig file.

author
#36 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 27 lines · David Miller

From: Lee Jones <[email protected]>
Date: Sun, 4 Nov 2012 11:53:32 +0100

Quoted 24 lines · Paul Bolle
Quoted 22 lines · Lee Jones
Quoted 11 lines · Paul Bolle
Quoted 6 lines · Lee Jones
Quoted 3 lines

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense.

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.)

Quoted 7 lines
Quoted 4 lines

I'm in the same position as you Paul. I just noticed the warning so fixed it following the current way of doing things. Any, more substantial changes requiring greater knowledge of the subsystem would have to be done by someone else.

I think the most appropriate thing to do is make CONFIG_ISDN depend upon CONFIG_NETDEVICES in the Kconfig file.

... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If that's suitable more suitable I can do that instead?

Just need a yay or nay and I'll make it happen.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
#37 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 5 lines · Lee Jones
Quoted 2 lines · David Miller

I think the most appropriate thing to do is make CONFIG_ISDN depend upon CONFIG_NETDEVICES in the Kconfig file.

... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends on ISDN. In that case CONFIG_NETDEVICES would always be true when compiling isdn_common.c. That would make these guards pointless. (The dependency of ISDN_PPP on NETDEVICES would then also be pointless.)

Paul Bolle

author
#38 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

Quoted 11 lines · Paul Bolle
Quoted 5 lines · Lee Jones
Quoted 2 lines · David Miller

I think the most appropriate thing to do is make CONFIG_ISDN depend upon CONFIG_NETDEVICES in the Kconfig file.

... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If ISDN would depend on NETDEVICES, ISDN_I4L would too, since it depends on ISDN. In that case CONFIG_NETDEVICES would always be true when compiling isdn_common.c. That would make these guards pointless. (The dependency of ISDN_PPP on NETDEVICES would then also be pointless.)

I'll submit a patch based on your comments.

Thanks for your time Paul.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
maintainer
#39 lore raw

Re: [PATCH 8/9] isdn: Remove unused variable causing a compile build warning

In reply to Lee Jones

From: Lee Jones <[email protected]>
Date: Mon, 5 Nov 2012 09:44:19 +0100

Quoted 34 lines · David Miller
Quoted 33 lines

From: Lee Jones <[email protected]>
Date: Sun, 4 Nov 2012 11:53:32 +0100

Quoted 28 lines · Paul Bolle
Quoted 14 lines · Lee Jones
Quoted 11 lines · Paul Bolle
Quoted 6 lines · Lee Jones
Quoted 3 lines

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

Did you have CONFIG_NETDEVICES not set in this build?

Ah yes, I see it. The function went down further than I thought it did. So the real fix is to ensure 's' is defined inside of some ifdef CONFIG_NETDEVICES guards.

Quoted 8 lines

What puzzles me is that we only find these "#ifdef CONFIG_NETDEVICES" guards in this file and not in isdn_net.c, were all the ioctl commands guarded that way seem to be calling into. On first glance that doesn't make much sense.

(Actually the idea of having ISDN without NETDEVICES is a bit puzzling too. But there are too many parts of the isdn subsystem that I'm unfamiliar with to say whether that can make sense.)

I'm in the same position as you Paul. I just noticed the warning so fixed it following the current way of doing things. Any, more substantial changes requiring greater knowledge of the subsystem would have to be done by someone else.

Quoted 8 lines
Quoted 2 lines

I think the most appropriate thing to do is make CONFIG_ISDN depend upon CONFIG_NETDEVICES in the Kconfig file.

... and then remove the CONFIG_NETDEVICES guards in isdn_common.c?

If that's suitable more suitable I can do that instead?

Just need a yay or nay and I'll make it happen.

"yay"

author
#40 lore raw

PATCH 8/9 isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

In reply to David Miller

Does something like look like a better solution?

Author: Lee Jones <[email protected]>
Date:   Sat Nov 3 22:06:02 2012 +0100

isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

It doesn't make much sense to enable ISDN services if you don't intend to connect to a network. Therefore insisting that ISDN depends on NETDEVICES seems logical. We can then remove any guards mentioning NETDEVICES inside all subordinate drivers.

This also has the nice side-effect of fixing the warning below when ISDN_I4L && !CONFIG_NETDEVICES at compile time.

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

drivers/isdn/Kconfig +1 -1
index a233ed5..86cd75a 100644
@@ -4,7 +4,7 @@
 
 menuconfig ISDN
 	bool "ISDN support"
-	depends on NET
+	depends on NET && NETDEVICES
 	depends on !S390 && !UML
 	---help---
 	  ISDN ("Integrated Services Digital Network", called RNIS in France)
drivers/isdn/i4l/Kconfig +1 -1
index 2302fbe..9c6650e 100644
@@ -6,7 +6,7 @@ if ISDN_I4L
 
 config ISDN_PPP
 	bool "Support synchronous PPP"
-	depends on INET && NETDEVICES
+	depends on INET
 	select SLHC
 	help
 	  Over digital connections such as ISDN, there is no need to
drivers/isdn/i4l/isdn_common.c +0 -4
index 8c610fa..e2a945e 100644
@@ -1312,7 +1312,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 			} else
 				return -EINVAL;
 			break;
-#ifdef CONFIG_NETDEVICES
 		case IIOCNETGPN:
 			/* Get peer phone number of a connected
 			 * isdn network interface */
@@ -1322,7 +1321,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 				return isdn_net_getpeer(&phone, argp);
 			} else
 				return -EINVAL;
-#endif
 		default:
 			return -EINVAL;
 		}
@@ -1352,7 +1350,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 		case IIOCNETLCR:
 			printk(KERN_INFO "INFO: ISDN_ABC_LCR_SUPPORT not enabled\n");
 			return -ENODEV;
-#ifdef CONFIG_NETDEVICES
 		case IIOCNETAIF:
 			/* Add a network-interface */
 			if (arg) {
@@ -1491,7 +1488,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 				return -EFAULT;
 			return isdn_net_force_hangup(name);
 			break;
-#endif                          /* CONFIG_NETDEVICES */
 		case IIOCSETVER:
 			dev->net_verbose = arg;
 			printk(KERN_INFO "isdn: Verbose-Level is %d\n", dev->net_verbose);
maintainer
#41 lore raw

Re: [PATCH 8/9] isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

From: Lee Jones <[email protected]>
Date: Mon, 5 Nov 2012 11:31:26 +0100

Quoted 6 lines

Does something like look like a better solution?

Author: Lee Jones <[email protected]>
Date:   Sat Nov 3 22:06:02 2012 +0100

isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

Yes, it looks good, please resubmit it formally.

author
#42 lore raw

Re: [PATCH 8/9] isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

Quoted 11 lines · David Miller

From: Lee Jones <[email protected]>
Date: Mon, 5 Nov 2012 11:31:26 +0100

Quoted 6 lines

Does something like look like a better solution?

Author: Lee Jones <[email protected]>
Date:   Sat Nov 3 22:06:02 2012 +0100

isdn: Make CONFIG_ISDN depend on CONFIG_NETDEVICES

Yes, it looks good, please resubmit it formally.

Done. Thanks for reviewing.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
author
#43 lore raw

PATCH v2 8/9 isdn: Remove unused variable causing a compile build warning

In reply to Paul Bolle

Thanks for reviewing Paul. Here's the result:

Author: Lee Jones <[email protected]>
Date:   Sat Nov 3 22:06:02 2012 +0100

isdn: Encapsulate variable 's' in same CONFIG_NETDEVICES guards as code using it

In the current case, variable 's' of type 'char *' is defined but then not used if !CONFIG_NETDEVICES, casing the compile-time warning below. In this change we surround the declaration using the same guards as the pre-processed code which makes use of it.

This patch fixes:
drivers/isdn/i4l/isdn_common.c: In function ‘isdn_ioctl’:
drivers/isdn/i4l/isdn_common.c:1278:8: warning: unused variable ‘s’ [-Wunused-variable]

drivers/isdn/i4l/isdn_common.c +3 -1
index 8c610fa..2875f31 100644
@@ -1275,7 +1275,6 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 	int ret;
 	int i;
 	char __user *p;
-	char *s;
 	union iocpar {
 		char name[10];
 		char bname[22];
@@ -1284,6 +1283,9 @@ isdn_ioctl(struct file *file, uint cmd, ulong arg)
 		isdn_net_ioctl_cfg cfg;
 	} iocpar;
 	void __user *argp = (void __user *)arg;
+#ifdef CONFIG_NETDEVICES
+	char *s;
+#endif
 
 #define name  iocpar.name
 #define bname iocpar.bname
author
#44 lore raw

PATCH 9/9 Avoid 'statement with no effect' compiler warnings

In reply to Lee Jones

Instead of issuing (0) statements when !CONFIG_SYSFS which will cause 'warning: ', we'll use inline statements instead. This will effectively do the same thing, but suppress any unnecessary warnings.

---
 net/bridge/br_private.h |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
net/bridge/br_private.h +4 -4
index 9b278c4..af5f584 100644
@@ -566,10 +566,10 @@ extern void br_sysfs_delbr(struct net_device *dev);
 
 #else
 
-#define br_sysfs_addif(p)	(0)
-#define br_sysfs_renameif(p)	(0)
-#define br_sysfs_addbr(dev)	(0)
-#define br_sysfs_delbr(dev)	do { } while(0)
+static inline int br_sysfs_addif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_renameif(struct net_bridge_port *p) { return 0; }
+static inline int br_sysfs_addbr(struct net_device *dev) { return 0; }
+static inline void br_sysfs_delbr(struct net_device *dev) { return; }
 #endif /* CONFIG_SYSFS */
 
 #endif
Signature · 1 line
1.7.9.5
maintainer Applied
#45 lore raw

Re: [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

From: Lee Jones <[email protected]>
Date: Sat, 3 Nov 2012 23:02:30 +0100

Quoted 8 lines

Instead of issuing (0) statements when !CONFIG_SYSFS which will cause 'warning: ', we'll use inline statements instead. This will effectively do the same thing, but suppress any unnecessary warnings.

Applied, but please use more informative subject lines.

You should prefix your subject line after [PATCH ...] with the name of the subsystem you are touching, a ": " then the headline description.

So here you would have used "bridge: " and that's what I added when I commited this patch.

author
#46 lore raw

Re: [PATCH 9/9] Avoid 'statement with no effect' compiler warnings

Quoted 12 lines · David Miller

From: Lee Jones <[email protected]>
Date: Sat, 3 Nov 2012 23:02:30 +0100

Quoted 8 lines

Instead of issuing (0) statements when !CONFIG_SYSFS which will cause 'warning: ', we'll use inline statements instead. This will effectively do the same thing, but suppress any unnecessary warnings.

Quoted 8 lines

Applied, but please use more informative subject lines.

You should prefix your subject line after [PATCH ...] with the name of the subsystem you are touching, a ": " then the headline description.

So here you would have used "bridge: " and that's what I added when I commited this patch.

Yes, of course I should have done, and usually do.

This was an oversight, sorry about that.

Signature · 4 lines
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog