public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] fix calls to request_module()
@ 2008-12-11  3:35 Nguyen Anh Quynh
  2008-12-11  4:01 ` Al Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  3:35 UTC (permalink / raw)
  To: Andrew Morton, LKML; +Cc: Kuniyasu Suzaki

[-- Attachment #1: Type: text/plain, Size: 3177 bytes --]

Hi,

The request_module() function should always have the 1st param as a
format argument. So for example, request_module("i2c-powermac") should
be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
patch fixes them all in linus-git tree.

Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com>

#diffstat fix-request_module-call.linus_git_tree.patch
 arch/arm/mach-pxa/am200epd.c                  |    2 +-
 crypto/api.c                                  |    2 +-
 drivers/block/paride/paride.c                 |    2 +-
 drivers/hid/hid-core.c                        |    2 +-
 drivers/ide/ide-probe.c                       |   10 +++++-----
 drivers/macintosh/therm_adt746x.c             |    2 +-
 drivers/macintosh/windfarm_pm112.c            |   12 ++++++------
 drivers/macintosh/windfarm_pm121.c            |   12 ++++++------
 drivers/macintosh/windfarm_pm81.c             |    8 ++++----
 drivers/macintosh/windfarm_pm91.c             |    8 ++++----
 drivers/media/video/bt8xx/bttv-cards.c        |   12 ++++++------
 drivers/media/video/cafe_ccic.c               |    2 +-
 drivers/media/video/cx18/cx18-driver.c        |    2 +-
 drivers/media/video/cx23885/cx23885-cards.c   |    4 ++--
 drivers/media/video/cx88/cx88-cards.c         |    2 +-
 drivers/media/video/cx88/cx88-mpeg.c          |    4 ++--
 drivers/media/video/cx88/cx88-video.c         |    6 +++---
 drivers/media/video/em28xx/em28xx-cards.c     |   12 ++++++------
 drivers/media/video/em28xx/em28xx-video.c     |    6 +++---
 drivers/media/video/ivtv/ivtv-driver.c        |    2 +-
 drivers/media/video/mxb.c                     |   10 +++++-----
 drivers/media/video/pvrusb2/pvrusb2-hdw.c     |    2 +-
 drivers/media/video/saa7134/saa7134-core.c    |   14 +++++++-------
 drivers/media/video/usbvision/usbvision-i2c.c |    6 +++---
 drivers/media/video/vino.c                    |    4 ++--
 drivers/media/video/w9968cf.c                 |    2 +-
 drivers/media/video/zoran/zoran_card.c        |    8 ++++----
 drivers/mtd/chips/gen_probe.c                 |    2 +-
 drivers/net/wireless/b43/rfkill.c             |    2 +-
 drivers/net/wireless/b43legacy/rfkill.c       |    2 +-
 drivers/net/wireless/hostap/hostap_ioctl.c    |   10 +++++-----
 drivers/of/of_spi.c                           |    2 +-
 drivers/parport/share.c                       |    2 +-
 drivers/usb/misc/ftdi-elan.c                  |    2 +-
 drivers/usb/storage/libusual.c                |    2 +-
 drivers/video/n411.c                          |    2 +-
 fs/dquot.c                                    |    2 +-
 fs/gfs2/locking.c                             |    2 +-
 net/atm/ioctl.c                               |   10 +++++-----
 net/ieee80211/ieee80211_wx.c                  |    4 ++--
 net/socket.c                                  |    6 +++---
 sound/core/sound.c                            |    2 +-
 sound/oss/sequencer.c                         |    2 +-
 sound/ppc/daca.c                              |    2 +-
 sound/ppc/tumbler.c                           |    2 +-
 45 files changed, 108 insertions(+), 108 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-request_module-call.linus_git_tree.patch --]
[-- Type: text/x-patch; name=fix-request_module-call.linus_git_tree.patch, Size: 30982 bytes --]

diff --git a/arch/arm/mach-pxa/am200epd.c b/arch/arm/mach-pxa/am200epd.c
index b965085..60910f4 100644
--- a/arch/arm/mach-pxa/am200epd.c
+++ b/arch/arm/mach-pxa/am200epd.c
@@ -340,7 +340,7 @@ static int __init am200_init(void)
 	fb_register_client(&am200_fb_notif);
 
 	/* request our platform independent driver */
-	request_module("metronomefb");
+	request_module("%s", "metronomefb");
 
 	am200_device = platform_device_alloc("metronomefb", -1);
 	if (!am200_device)
diff --git a/crypto/api.c b/crypto/api.c
index 0444d24..e179355 100644
--- a/crypto/api.c
+++ b/crypto/api.c
@@ -230,7 +230,7 @@ int crypto_probing_notify(unsigned long val, void *v)
 
 	ok = blocking_notifier_call_chain(&crypto_chain, val, v);
 	if (ok == NOTIFY_DONE) {
-		request_module("cryptomgr");
+		request_module("%s", "cryptomgr");
 		ok = blocking_notifier_call_chain(&crypto_chain, val, v);
 	}
 
diff --git a/drivers/block/paride/paride.c b/drivers/block/paride/paride.c
index 48c50f1..437cfbf 100644
--- a/drivers/block/paride/paride.c
+++ b/drivers/block/paride/paride.c
@@ -355,7 +355,7 @@ int pi_init(PIA * pi, int autoprobe, int port, int mode,
 	e = s + 1;
 
 	if (!protocols[0])
-		request_module("paride_protocol");
+		request_module("%s", "paride_protocol");
 
 	if (autoprobe) {
 		s = 0;
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 40df3e1..6bbad79 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1713,7 +1713,7 @@ EXPORT_SYMBOL_GPL(hid_unregister_driver);
 #ifdef CONFIG_HID_COMPAT
 static void hid_compat_load(struct work_struct *ws)
 {
-	request_module("hid-dummy");
+	request_module("%s", "hid-dummy");
 }
 static DECLARE_WORK(hid_compat_work, hid_compat_load);
 static struct workqueue_struct *hid_compat_wq;
diff --git a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
index c55bdbd..dadd26f 100644
--- a/drivers/ide/ide-probe.c
+++ b/drivers/ide/ide-probe.c
@@ -1179,15 +1179,15 @@ static struct kobject *ata_probe(dev_t dev, int *part, void *data)
 		return NULL;
 
 	if (drive->media == ide_disk)
-		request_module("ide-disk");
+		request_module("%s", "ide-disk");
 	if (drive->dev_flags & IDE_DFLAG_SCSI)
-		request_module("ide-scsi");
+		request_module("%s", "ide-scsi");
 	if (drive->media == ide_cdrom || drive->media == ide_optical)
-		request_module("ide-cd");
+		request_module("%s", "ide-cd");
 	if (drive->media == ide_tape)
-		request_module("ide-tape");
+		request_module("%s", "ide-tape");
 	if (drive->media == ide_floppy)
-		request_module("ide-floppy");
+		request_module("%s", "ide-floppy");
 
 	return NULL;
 }
diff --git a/drivers/macintosh/therm_adt746x.c b/drivers/macintosh/therm_adt746x.c
index 22bf981..f085ebe 100644
--- a/drivers/macintosh/therm_adt746x.c
+++ b/drivers/macintosh/therm_adt746x.c
@@ -636,7 +636,7 @@ thermostat_init(void)
 			"Failed to create tempertaure attribute file(s).\n");
 
 #ifndef CONFIG_I2C_POWERMAC
-	request_module("i2c-powermac");
+	request_module("%s", "i2c-powermac");
 #endif
 
 	return i2c_add_driver(&thermostat_driver);
diff --git a/drivers/macintosh/windfarm_pm112.c b/drivers/macintosh/windfarm_pm112.c
index 73d695d..2955f44 100644
--- a/drivers/macintosh/windfarm_pm112.c
+++ b/drivers/macintosh/windfarm_pm112.c
@@ -687,12 +687,12 @@ static int __init wf_pm112_init(void)
 	printk(KERN_INFO "windfarm: initializing for dual-core desktop G5\n");
 
 #ifdef MODULE
-	request_module("windfarm_smu_controls");
-	request_module("windfarm_smu_sensors");
-	request_module("windfarm_smu_sat");
-	request_module("windfarm_lm75_sensor");
-	request_module("windfarm_max6690_sensor");
-	request_module("windfarm_cpufreq_clamp");
+	request_module("%s", "windfarm_smu_controls");
+	request_module("%s", "windfarm_smu_sensors");
+	request_module("%s", "windfarm_smu_sat");
+	request_module("%s", "windfarm_lm75_sensor");
+	request_module("%s", "windfarm_max6690_sensor");
+	request_module("%s", "windfarm_cpufreq_clamp");
 
 #endif /* MODULE */
 
diff --git a/drivers/macintosh/windfarm_pm121.c b/drivers/macintosh/windfarm_pm121.c
index 66ec4fb..858dc9a 100644
--- a/drivers/macintosh/windfarm_pm121.c
+++ b/drivers/macintosh/windfarm_pm121.c
@@ -1012,12 +1012,12 @@ static int __init pm121_init(void)
 		rc = pm121_init_pm();
 
 	if (rc == 0) {
-		request_module("windfarm_smu_controls");
-		request_module("windfarm_smu_sensors");
-		request_module("windfarm_smu_sat");
-		request_module("windfarm_lm75_sensor");
-		request_module("windfarm_max6690_sensor");
-		request_module("windfarm_cpufreq_clamp");
+		request_module("%s", "windfarm_smu_controls");
+		request_module("%s", "windfarm_smu_sensors");
+		request_module("%s", "windfarm_smu_sat");
+		request_module("%s", "windfarm_lm75_sensor");
+		request_module("%s", "windfarm_max6690_sensor");
+		request_module("%s", "windfarm_cpufreq_clamp");
 		platform_driver_register(&pm121_driver);
 	}
 
diff --git a/drivers/macintosh/windfarm_pm81.c b/drivers/macintosh/windfarm_pm81.c
index abbe206..bb9571f 100644
--- a/drivers/macintosh/windfarm_pm81.c
+++ b/drivers/macintosh/windfarm_pm81.c
@@ -785,10 +785,10 @@ static int __init wf_smu_init(void)
 
 	if (rc == 0) {
 #ifdef MODULE
-		request_module("windfarm_smu_controls");
-		request_module("windfarm_smu_sensors");
-		request_module("windfarm_lm75_sensor");
-		request_module("windfarm_cpufreq_clamp");
+		request_module("%s", "windfarm_smu_controls");
+		request_module("%s", "windfarm_smu_sensors");
+		request_module("%s", "windfarm_lm75_sensor");
+		request_module("%s", "windfarm_cpufreq_clamp");
 
 #endif /* MODULE */
 		platform_driver_register(&wf_smu_driver);
diff --git a/drivers/macintosh/windfarm_pm91.c b/drivers/macintosh/windfarm_pm91.c
index 764c525..b089195 100644
--- a/drivers/macintosh/windfarm_pm91.c
+++ b/drivers/macintosh/windfarm_pm91.c
@@ -716,10 +716,10 @@ static int __init wf_smu_init(void)
 
 	if (rc == 0) {
 #ifdef MODULE
-		request_module("windfarm_smu_controls");
-		request_module("windfarm_smu_sensors");
-		request_module("windfarm_lm75_sensor");
-		request_module("windfarm_cpufreq_clamp");
+		request_module("%s", "windfarm_smu_controls");
+		request_module("%s", "windfarm_smu_sensors");
+		request_module("%s", "windfarm_lm75_sensor");
+		request_module("%s", "windfarm_cpufreq_clamp");
 
 #endif /* MODULE */
 		platform_driver_register(&wf_smu_driver);
diff --git a/drivers/media/video/bt8xx/bttv-cards.c b/drivers/media/video/bt8xx/bttv-cards.c
index 13742b0..5aaaccc 100644
--- a/drivers/media/video/bt8xx/bttv-cards.c
+++ b/drivers/media/video/bt8xx/bttv-cards.c
@@ -3689,25 +3689,25 @@ void __devinit bttv_init_card2(struct bttv *btv)
 	/* try to detect audio/fader chips */
 	if (!bttv_tvcards[btv->c.type].no_msp34xx &&
 	    bttv_I2CRead(btv, I2C_ADDR_MSP3400, "MSP34xx") >=0)
-		request_module("msp3400");
+		request_module("%s", "msp3400");
 
 	if (bttv_tvcards[btv->c.type].msp34xx_alt &&
 	    bttv_I2CRead(btv, I2C_ADDR_MSP3400_ALT, "MSP34xx (alternate address)") >=0)
-		request_module("msp3400");
+		request_module("%s", "msp3400");
 
 	if (!bttv_tvcards[btv->c.type].no_tda9875 &&
 	    bttv_I2CRead(btv, I2C_ADDR_TDA9875, "TDA9875") >=0)
-		request_module("tda9875");
+		request_module("%s", "tda9875");
 
 	if (!bttv_tvcards[btv->c.type].no_tda7432 &&
 	    bttv_I2CRead(btv, I2C_ADDR_TDA7432, "TDA7432") >=0)
-		request_module("tda7432");
+		request_module("%s", "tda7432");
 
 	if (bttv_tvcards[btv->c.type].needs_tvaudio)
-		request_module("tvaudio");
+		request_module("%s", "tvaudio");
 
 	if (btv->tuner_type != UNSET && btv->tuner_type != TUNER_ABSENT)
-		request_module("tuner");
+		request_module("%s", "tuner");
 }
 
 
diff --git a/drivers/media/video/cafe_ccic.c b/drivers/media/video/cafe_ccic.c
index 1740b9e..3c59b55 100644
--- a/drivers/media/video/cafe_ccic.c
+++ b/drivers/media/video/cafe_ccic.c
@@ -2314,7 +2314,7 @@ static int __init cafe_init(void)
 		printk(KERN_ERR "Unable to register cafe_ccic driver\n");
 		goto out;
 	}
-	request_module("ov7670");  /* FIXME want something more general */
+	request_module("%s", "ov7670");  /* FIXME want something more general */
 	ret = 0;
 
   out:
diff --git a/drivers/media/video/cx18/cx18-driver.c b/drivers/media/video/cx18/cx18-driver.c
index 7874d97..81f0d5e 100644
--- a/drivers/media/video/cx18/cx18-driver.c
+++ b/drivers/media/video/cx18/cx18-driver.c
@@ -572,7 +572,7 @@ static u32 cx18_request_module(struct cx18 *cx, u32 hw,
 {
 	if ((hw & id) == 0)
 		return hw;
-	if (request_module(name) != 0) {
+	if (request_module("%s", name) != 0) {
 		CX18_ERR("Failed to load module %s\n", name);
 		return hw & ~id;
 	}
diff --git a/drivers/media/video/cx23885/cx23885-cards.c b/drivers/media/video/cx23885/cx23885-cards.c
index dac5ccc..a9860a4 100644
--- a/drivers/media/video/cx23885/cx23885-cards.c
+++ b/drivers/media/video/cx23885/cx23885-cards.c
@@ -557,7 +557,7 @@ int cx23885_ir_init(struct cx23885_dev *dev)
 		/* FIXME: Implement me */
 		break;
 	case CX23885_BOARD_DVICO_FUSIONHDTV_DVB_T_DUAL_EXP:
-		request_module("ir-kbd-i2c");
+		request_module("%s", "ir-kbd-i2c");
 		break;
 	}
 
@@ -644,7 +644,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
 	case CX23885_BOARD_HAUPPAUGE_HVR1800lp:
 	case CX23885_BOARD_HAUPPAUGE_HVR1700:
 	case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H:
-		request_module("cx25840");
+		request_module("%s", "cx25840");
 		break;
 	}
 }
diff --git a/drivers/media/video/cx88/cx88-cards.c b/drivers/media/video/cx88/cx88-cards.c
index 5bcbb4c..1c371b9 100644
--- a/drivers/media/video/cx88/cx88-cards.c
+++ b/drivers/media/video/cx88/cx88-cards.c
@@ -3068,7 +3068,7 @@ struct cx88_core *cx88_core_create(struct pci_dev *pci, int nr)
 
 	/* load tuner module, if needed */
 	if (TUNER_ABSENT != core->board.tuner_type)
-		request_module("tuner");
+		request_module("%s", "tuner");
 
 	cx88_card_setup(core);
 	cx88_ir_init(core, pci);
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 3ebdcd1..7cd9162 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -55,9 +55,9 @@ static void request_module_async(struct work_struct *work)
 	struct cx8802_dev *dev=container_of(work, struct cx8802_dev, request_module_wk);
 
 	if (dev->core->board.mpeg & CX88_MPEG_DVB)
-		request_module("cx88-dvb");
+		request_module("%s", "cx88-dvb");
 	if (dev->core->board.mpeg & CX88_MPEG_BLACKBIRD)
-		request_module("cx88-blackbird");
+		request_module("%s", "cx88-blackbird");
 }
 
 static void request_modules(struct cx8802_dev *dev)
diff --git a/drivers/media/video/cx88/cx88-video.c b/drivers/media/video/cx88/cx88-video.c
index b96ce99..c157989 100644
--- a/drivers/media/video/cx88/cx88-video.c
+++ b/drivers/media/video/cx88/cx88-video.c
@@ -1895,15 +1895,15 @@ static int __devinit cx8800_initdev(struct pci_dev *pci_dev,
 	/* load and configure helper modules */
 
 	if (core->board.audio_chip == V4L2_IDENT_WM8775)
-		request_module("wm8775");
+		request_module("%s", "wm8775");
 
 	switch (core->boardnr) {
 	case CX88_BOARD_DVICO_FUSIONHDTV_5_GOLD:
 	case CX88_BOARD_DVICO_FUSIONHDTV_7_GOLD:
-		request_module("rtc-isl1208");
+		request_module("%s", "rtc-isl1208");
 		/* break intentionally omitted */
 	case CX88_BOARD_DVICO_FUSIONHDTV_5_PCI_NANO:
-		request_module("ir-kbd-i2c");
+		request_module("%s", "ir-kbd-i2c");
 	}
 
 	/* register v4l devices */
diff --git a/drivers/media/video/em28xx/em28xx-cards.c b/drivers/media/video/em28xx/em28xx-cards.c
index d65d057..e8c9c02 100644
--- a/drivers/media/video/em28xx/em28xx-cards.c
+++ b/drivers/media/video/em28xx/em28xx-cards.c
@@ -1722,7 +1722,7 @@ void em28xx_card_setup(struct em28xx *dev)
 	{
 		struct tveeprom tv;
 #ifdef CONFIG_MODULES
-		request_module("tveeprom");
+		request_module("%s", "tveeprom");
 #endif
 		/* Call first TVeeprom */
 
@@ -1737,7 +1737,7 @@ void em28xx_card_setup(struct em28xx *dev)
 		}
 #ifdef CONFIG_MODULES
 		if (tv.has_ir)
-			request_module("ir-kbd-i2c");
+			request_module("%s", "ir-kbd-i2c");
 #endif
 		break;
 	}
@@ -1785,13 +1785,13 @@ void em28xx_card_setup(struct em28xx *dev)
 #ifdef CONFIG_MODULES
 	/* request some modules */
 	if (dev->has_msp34xx)
-		request_module("msp3400");
+		request_module("%s", "msp3400");
 	if (dev->decoder == EM28XX_SAA7113 || dev->decoder == EM28XX_SAA7114)
-		request_module("saa7115");
+		request_module("%s", "saa7115");
 	if (dev->decoder == EM28XX_TVP5150)
-		request_module("tvp5150");
+		request_module("%s", "tvp5150");
 	if (dev->tuner_type != TUNER_ABSENT)
-		request_module("tuner");
+		request_module("%s", "tuner");
 #endif
 
 	em28xx_config_tuner(dev);
diff --git a/drivers/media/video/em28xx/em28xx-video.c b/drivers/media/video/em28xx/em28xx-video.c
index 610f535..2299476 100644
--- a/drivers/media/video/em28xx/em28xx-video.c
+++ b/drivers/media/video/em28xx/em28xx-video.c
@@ -2117,12 +2117,12 @@ static void request_module_async(struct work_struct *work)
 			     struct em28xx, request_module_wk);
 
 	if (dev->has_audio_class)
-		request_module("snd-usb-audio");
+		request_module("%s", "snd-usb-audio");
 	else
-		request_module("em28xx-alsa");
+		request_module("%s", "em28xx-alsa");
 
 	if (dev->has_dvb)
-		request_module("em28xx-dvb");
+		request_module("%s", "em28xx-dvb");
 }
 
 static void request_modules(struct em28xx *dev)
diff --git a/drivers/media/video/ivtv/ivtv-driver.c b/drivers/media/video/ivtv/ivtv-driver.c
index b69cc1d..a26ad59 100644
--- a/drivers/media/video/ivtv/ivtv-driver.c
+++ b/drivers/media/video/ivtv/ivtv-driver.c
@@ -859,7 +859,7 @@ static u32 ivtv_request_module(struct ivtv *itv, u32 hw,
 {
 	if ((hw & id) == 0)
 		return hw;
-	if (request_module(name) != 0) {
+	if (request_module("%s", name) != 0) {
 		IVTV_ERR("Failed to load module %s\n", name);
 		return hw & ~id;
 	}
diff --git a/drivers/media/video/mxb.c b/drivers/media/video/mxb.c
index 7f13028..6614e40 100644
--- a/drivers/media/video/mxb.c
+++ b/drivers/media/video/mxb.c
@@ -181,27 +181,27 @@ static int mxb_probe(struct saa7146_dev* dev)
 	struct mxb* mxb = NULL;
 	int result;
 
-	result = request_module("saa7115");
+	result = request_module("%s", "saa7115");
 	if (result < 0) {
 		printk("mxb: saa7111 i2c module not available.\n");
 		return -ENODEV;
 	}
-	result = request_module("tea6420");
+	result = request_module("%s", "tea6420");
 	if (result < 0) {
 		printk("mxb: tea6420 i2c module not available.\n");
 		return -ENODEV;
 	}
-	result = request_module("tea6415c");
+	result = request_module("%s", "tea6415c");
 	if (result < 0) {
 		printk("mxb: tea6415c i2c module not available.\n");
 		return -ENODEV;
 	}
-	result = request_module("tda9840");
+	result = request_module("%s", "tda9840");
 	if (result < 0) {
 		printk("mxb: tda9840 i2c module not available.\n");
 		return -ENODEV;
 	}
-	result = request_module("tuner");
+	result = request_module("%s", "tuner");
 	if (result < 0) {
 		printk("mxb: tuner i2c module not available.\n");
 		return -ENODEV;
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index 5b81ba4..6d75d3c 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -1967,7 +1967,7 @@ static void pvr2_hdw_setup_low(struct pvr2_hdw *hdw)
 	if (!pvr2_hdw_dev_ok(hdw)) return;
 
 	for (idx = 0; idx < hdw->hdw_desc->client_modules.cnt; idx++) {
-		request_module(hdw->hdw_desc->client_modules.lst[idx]);
+		request_module("%s", hdw->hdw_desc->client_modules.lst[idx]);
 	}
 
 	if (!hdw->hdw_desc->flag_no_powerup) {
diff --git a/drivers/media/video/saa7134/saa7134-core.c b/drivers/media/video/saa7134/saa7134-core.c
index dfbe08a..ff0408e 100644
--- a/drivers/media/video/saa7134/saa7134-core.c
+++ b/drivers/media/video/saa7134/saa7134-core.c
@@ -153,13 +153,13 @@ void saa7134_set_gpio(struct saa7134_dev *dev, int bit_no, int value)
 static void request_module_async(struct work_struct *work){
 	struct saa7134_dev* dev = container_of(work, struct saa7134_dev, request_module_wk);
 	if (card_is_empress(dev))
-		request_module("saa7134-empress");
+		request_module("%s", "saa7134-empress");
 	if (card_is_dvb(dev))
-		request_module("saa7134-dvb");
+		request_module("%s", "saa7134-dvb");
 	if (alsa)
-		request_module("saa7134-alsa");
+		request_module("%s", "saa7134-alsa");
 	if (oss)
-		request_module("saa7134-oss");
+		request_module("%s", "saa7134-oss");
 }
 
 static void request_submodules(struct saa7134_dev *dev)
@@ -700,7 +700,7 @@ static int saa7134_hw_enable2(struct saa7134_dev *dev)
 	}
 
 	if (dev->has_remote == SAA7134_REMOTE_I2C) {
-		request_module("ir-kbd-i2c");
+		request_module("%s", "ir-kbd-i2c");
 	}
 
 	saa_writel(SAA7134_IRQ1, 0);
@@ -970,14 +970,14 @@ static int __devinit saa7134_initdev(struct pci_dev *pci_dev,
 
 	/* initialize hardware #2 */
 	if (TUNER_ABSENT != dev->tuner_type)
-		request_module("tuner");
+		request_module("%s", "tuner");
 	saa7134_board_init2(dev);
 
 	saa7134_hwinit2(dev);
 
 	/* load i2c helpers */
 	if (card_is_empress(dev)) {
-		request_module("saa6752hs");
+		request_module("%s", "saa6752hs");
 	}
 
 	request_submodules(dev);
diff --git a/drivers/media/video/usbvision/usbvision-i2c.c b/drivers/media/video/usbvision/usbvision-i2c.c
index 9907b9a..49eeeb1 100644
--- a/drivers/media/video/usbvision/usbvision-i2c.c
+++ b/drivers/media/video/usbvision/usbvision-i2c.c
@@ -254,14 +254,14 @@ int usbvision_i2c_register(struct usb_usbvision *usbvision)
 	/* Request the load of the i2c modules we need */
 	switch (usbvision_device_data[usbvision->DevModel].Codec) {
 	case CODEC_SAA7113:
-		request_module("saa7115");
+		request_module("%s", "saa7115");
 		break;
 	case CODEC_SAA7111:
-		request_module("saa7115");
+		request_module("%s", "saa7115");
 		break;
 	}
 	if (usbvision_device_data[usbvision->DevModel].Tuner == 1) {
-		request_module("tuner");
+		request_module("%s", "tuner");
 	}
 #endif
 
diff --git a/drivers/media/video/vino.c b/drivers/media/video/vino.c
index 1efc5f3..9f9fc94 100644
--- a/drivers/media/video/vino.c
+++ b/drivers/media/video/vino.c
@@ -4632,8 +4632,8 @@ static int __init vino_module_init(void)
 	vino_init_stage++;
 
 #ifdef MODULE
-	request_module("saa7191");
-	request_module("indycam");
+	request_module("%s", "saa7191");
+	request_module("%s", "indycam");
 #endif
 
 	dprintk("init complete!\n");
diff --git a/drivers/media/video/w9968cf.c b/drivers/media/video/w9968cf.c
index 4dfb43b..30056a0 100644
--- a/drivers/media/video/w9968cf.c
+++ b/drivers/media/video/w9968cf.c
@@ -3656,7 +3656,7 @@ static int __init w9968cf_module_init(void)
 	KDBG(3, W9968CF_MODULE_AUTHOR)
 
 	if (ovmod_load)
-		request_module("ovcamchip");
+		request_module("%s", "ovcamchip");
 
 	if ((err = usb_register(&w9968cf_usb_driver)))
 		return err;
diff --git a/drivers/media/video/zoran/zoran_card.c b/drivers/media/video/zoran/zoran_card.c
index fa5f2f8..586ee7d 100644
--- a/drivers/media/video/zoran/zoran_card.c
+++ b/drivers/media/video/zoran/zoran_card.c
@@ -1435,7 +1435,7 @@ find_zr36057 (void)
 		}
 
 		if (i2c_dec_name) {
-			if ((result = request_module(i2c_dec_name)) < 0) {
+			if ((result = request_module("%s", i2c_dec_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load module %s: %d\n",
@@ -1455,7 +1455,7 @@ find_zr36057 (void)
 		}
 
 		if (i2c_enc_name) {
-			if ((result = request_module(i2c_enc_name)) < 0) {
+			if ((result = request_module("%s", i2c_enc_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load module %s: %d\n",
@@ -1478,7 +1478,7 @@ find_zr36057 (void)
 		if (zr->card.video_codec != 0 &&
 		    (codec_name =
 		     codecid_to_modulename(zr->card.video_codec)) != NULL) {
-			if ((result = request_module(codec_name)) < 0) {
+			if ((result = request_module("%s", codec_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load modules %s: %d\n",
@@ -1488,7 +1488,7 @@ find_zr36057 (void)
 		if (zr->card.video_vfe != 0 &&
 		    (vfe_name =
 		     codecid_to_modulename(zr->card.video_vfe)) != NULL) {
-			if ((result = request_module(vfe_name)) < 0) {
+			if ((result = request_module("%s", vfe_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load modules %s: %d\n",
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index e2dc964..814d4f3 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -212,7 +212,7 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		request_module("%s", probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/drivers/net/wireless/b43/rfkill.c b/drivers/net/wireless/b43/rfkill.c
index 7137537..c314d4a 100644
--- a/drivers/net/wireless/b43/rfkill.c
+++ b/drivers/net/wireless/b43/rfkill.c
@@ -164,7 +164,7 @@ void b43_rfkill_init(struct b43_wldev *dev)
 #ifdef CONFIG_RFKILL_INPUT_MODULE
 	/* B43 RF-kill isn't useful without the rfkill-input subsystem.
 	 * Try to load the module. */
-	err = request_module("rfkill-input");
+	err = request_module("%s", "rfkill-input");
 	if (err)
 		b43warn(wl, "Failed to load the rfkill-input module. "
 			"The built-in radio LED will not work.\n");
diff --git a/drivers/net/wireless/b43legacy/rfkill.c b/drivers/net/wireless/b43legacy/rfkill.c
index b32bf6a..055dbff 100644
--- a/drivers/net/wireless/b43legacy/rfkill.c
+++ b/drivers/net/wireless/b43legacy/rfkill.c
@@ -167,7 +167,7 @@ void b43legacy_rfkill_init(struct b43legacy_wldev *dev)
 #ifdef CONFIG_RFKILL_INPUT_MODULE
 	/* B43legacy RF-kill isn't useful without the rfkill-input subsystem.
 	 * Try to load the module. */
-	err = request_module("rfkill-input");
+	err = request_module("%s", "rfkill-input");
 	if (err)
 		b43legacywarn(wl, "Failed to load the rfkill-input module."
 			"The built-in radio LED will not work.\n");
diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index 3f8b1d7..ea03992 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -186,7 +186,7 @@ static int prism2_ioctl_siwencode(struct net_device *dev,
 			return -ENOMEM;
 		new_crypt->ops = ieee80211_get_crypto_ops("WEP");
 		if (!new_crypt->ops) {
-			request_module("ieee80211_crypt_wep");
+			request_module("%s", "ieee80211_crypt_wep");
 			new_crypt->ops = ieee80211_get_crypto_ops("WEP");
 		}
 		if (new_crypt->ops)
@@ -3297,7 +3297,7 @@ static int prism2_ioctl_siwencodeext(struct net_device *dev,
 
 	ops = ieee80211_get_crypto_ops(alg);
 	if (ops == NULL) {
-		request_module(module);
+		request_module("%s", module);
 		ops = ieee80211_get_crypto_ops(alg);
 	}
 	if (ops == NULL) {
@@ -3511,13 +3511,13 @@ static int prism2_ioctl_set_encryption(local_info_t *local,
 
 	ops = ieee80211_get_crypto_ops(param->u.crypt.alg);
 	if (ops == NULL && strcmp(param->u.crypt.alg, "WEP") == 0) {
-		request_module("ieee80211_crypt_wep");
+		request_module("%s", "ieee80211_crypt_wep");
 		ops = ieee80211_get_crypto_ops(param->u.crypt.alg);
 	} else if (ops == NULL && strcmp(param->u.crypt.alg, "TKIP") == 0) {
-		request_module("ieee80211_crypt_tkip");
+		request_module("%s", "ieee80211_crypt_tkip");
 		ops = ieee80211_get_crypto_ops(param->u.crypt.alg);
 	} else if (ops == NULL && strcmp(param->u.crypt.alg, "CCMP") == 0) {
-		request_module("ieee80211_crypt_ccmp");
+		request_module("%s", "ieee80211_crypt_ccmp");
 		ops = ieee80211_get_crypto_ops(param->u.crypt.alg);
 	}
 	if (ops == NULL) {
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index bed0ed6..ad96437 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -82,7 +82,7 @@ void of_register_spi_devices(struct spi_master *master, struct device_node *np)
 		spi->dev.archdata.of_node = nc;
 
 		/* Register the new device */
-		request_module(spi->modalias);
+		request_module("%s", spi->modalias);
 		rc = spi_add_device(spi);
 		if (rc) {
 			dev_err(&master->dev, "spi_device register error %s\n",
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 0ebca45..eb5f47e 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -123,7 +123,7 @@ static void get_lowlevel_driver (void)
 {
 	/* There is no actual module called this: you should set
 	 * up an alias for modutils. */
-	request_module ("parport_lowlevel");
+	request_module ("%s", "parport_lowlevel");
 }
 
 /**
diff --git a/drivers/usb/misc/ftdi-elan.c b/drivers/usb/misc/ftdi-elan.c
index 79a7668..29a9a14 100644
--- a/drivers/usb/misc/ftdi-elan.c
+++ b/drivers/usb/misc/ftdi-elan.c
@@ -321,7 +321,7 @@ static int ftdi_elan_hcd_init(struct usb_ftdi *ftdi)
         snprintf(ftdi->device_name, sizeof(ftdi->device_name), "u132_hcd");
         ftdi->platform_dev.name = ftdi->device_name;
         dev_info(&ftdi->udev->dev, "requesting module '%s'\n", "u132_hcd");
-        request_module("u132_hcd");
+        request_module("%s", "u132_hcd");
         dev_info(&ftdi->udev->dev, "registering '%s'\n",
                 ftdi->platform_dev.name);
         result = platform_device_register(&ftdi->platform_dev);
diff --git a/drivers/usb/storage/libusual.c b/drivers/usb/storage/libusual.c
index d617e8a..a481e02 100644
--- a/drivers/usb/storage/libusual.c
+++ b/drivers/usb/storage/libusual.c
@@ -180,7 +180,7 @@ static int usu_probe_thread(void *arg)
 	unsigned long flags;
 
 	mutex_lock(&usu_probe_mutex);
-	rc = request_module(bias_names[type]);
+	rc = request_module("%s", bias_names[type]);
 	spin_lock_irqsave(&usu_lock, flags);
 	if (rc == 0 && (st->fls & USU_MOD_FL_PRESENT) == 0) {
 		/*
diff --git a/drivers/video/n411.c b/drivers/video/n411.c
index 935830f..9ef570c 100644
--- a/drivers/video/n411.c
+++ b/drivers/video/n411.c
@@ -159,7 +159,7 @@ static int __init n411_init(void)
 	}
 
 	/* request our platform independent driver */
-	request_module("hecubafb");
+	request_module("%s", "hecubafb");
 
 	n411_device = platform_device_alloc("hecubafb", -1);
 	if (!n411_device)
diff --git a/fs/dquot.c b/fs/dquot.c
index 5e95261..3984307 100644
--- a/fs/dquot.c
+++ b/fs/dquot.c
@@ -167,7 +167,7 @@ static struct quota_format_type *find_quota_format(int id)
 		spin_unlock(&dq_list_lock);
 		
 		for (qm = 0; module_names[qm].qm_fmt_id && module_names[qm].qm_fmt_id != id; qm++);
-		if (!module_names[qm].qm_fmt_id || request_module(module_names[qm].qm_mod_name))
+		if (!module_names[qm].qm_fmt_id || request_module("%s", module_names[qm].qm_mod_name))
 			return NULL;
 
 		spin_lock(&dq_list_lock);
diff --git a/fs/gfs2/locking.c b/fs/gfs2/locking.c
index 523243a..7a9df7f 100644
--- a/fs/gfs2/locking.c
+++ b/fs/gfs2/locking.c
@@ -177,7 +177,7 @@ retry:
 		if (!try && capable(CAP_SYS_MODULE)) {
 			try = 1;
 			mutex_unlock(&lmh_lock);
-			request_module(proto_name);
+			request_module("%s", proto_name);
 			goto retry;
 		}
 		printk(KERN_INFO "GFS2: can't find protocol %s\n", proto_name);
diff --git a/net/atm/ioctl.c b/net/atm/ioctl.c
index 7afd8e7..8ff46ba 100644
--- a/net/atm/ioctl.c
+++ b/net/atm/ioctl.c
@@ -118,23 +118,23 @@ int vcc_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 					goto done;
 				switch (backend) {
 					case ATM_BACKEND_PPP:
-						request_module("pppoatm");
+						request_module("%s", "pppoatm");
 						break;
 					case ATM_BACKEND_BR2684:
-						request_module("br2684");
+						request_module("%s", "br2684");
 						break;
 				}
 			}
 			break;
 		case ATMMPC_CTRL:
 		case ATMMPC_DATA:
-			request_module("mpoa");
+			request_module("%s", "mpoa");
 			break;
 		case ATMARPD_CTRL:
-			request_module("clip");
+			request_module("%s", "clip");
 			break;
 		case ATMLEC_CTRL:
-			request_module("lec");
+			request_module("%s", "lec");
 			break;
 	}
 
diff --git a/net/ieee80211/ieee80211_wx.c b/net/ieee80211/ieee80211_wx.c
index 973832d..de4be03 100644
--- a/net/ieee80211/ieee80211_wx.c
+++ b/net/ieee80211/ieee80211_wx.c
@@ -383,7 +383,7 @@ int ieee80211_wx_set_encode(struct ieee80211_device *ieee,
 			return -ENOMEM;
 		new_crypt->ops = ieee80211_get_crypto_ops("WEP");
 		if (!new_crypt->ops) {
-			request_module("ieee80211_crypt_wep");
+			request_module("%s", "ieee80211_crypt_wep");
 			new_crypt->ops = ieee80211_get_crypto_ops("WEP");
 		}
 
@@ -608,7 +608,7 @@ int ieee80211_wx_set_encodeext(struct ieee80211_device *ieee,
 
 	ops = ieee80211_get_crypto_ops(alg);
 	if (ops == NULL) {
-		request_module(module);
+		request_module("%s", module);
 		ops = ieee80211_get_crypto_ops(alg);
 	}
 	if (ops == NULL) {
diff --git a/net/socket.c b/net/socket.c
index 92764d8..0a46882 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -888,7 +888,7 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		case SIOCBRDELBR:
 			err = -ENOPKG;
 			if (!br_ioctl_hook)
-				request_module("bridge");
+				request_module("%s", "bridge");
 
 			mutex_lock(&br_ioctl_mutex);
 			if (br_ioctl_hook)
@@ -899,7 +899,7 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		case SIOCSIFVLAN:
 			err = -ENOPKG;
 			if (!vlan_ioctl_hook)
-				request_module("8021q");
+				request_module("%s", "8021q");
 
 			mutex_lock(&vlan_ioctl_mutex);
 			if (vlan_ioctl_hook)
@@ -910,7 +910,7 @@ static long sock_ioctl(struct file *file, unsigned cmd, unsigned long arg)
 		case SIOCDELDLCI:
 			err = -ENOPKG;
 			if (!dlci_ioctl_hook)
-				request_module("dlci");
+				request_module("%s", "dlci");
 
 			mutex_lock(&dlci_ioctl_mutex);
 			if (dlci_ioctl_hook)
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 44a69bb..c143b9c 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -88,7 +88,7 @@ static void snd_request_other(int minor)
 	case SNDRV_MINOR_TIMER:		str = "snd-timer";	break;
 	default:			return;
 	}
-	request_module(str);
+	request_module("%s", str);
 }
 
 #endif	/* modular kernel */
diff --git a/sound/oss/sequencer.c b/sound/oss/sequencer.c
index 5c215f7..a03cf6f 100644
--- a/sound/oss/sequencer.c
+++ b/sound/oss/sequencer.c
@@ -965,7 +965,7 @@ int sequencer_open(int dev, struct file *file)
 		return -ENXIO;
 
 	if(synth_devs[dev] == NULL)
-		request_module("synth0");
+		request_module("%s", "synth0");
 
 	if (mode == OPEN_READ)
 	{
diff --git a/sound/ppc/daca.c b/sound/ppc/daca.c
index 8a5b290..c1391b6 100644
--- a/sound/ppc/daca.c
+++ b/sound/ppc/daca.c
@@ -249,7 +249,7 @@ int __init snd_pmac_daca_init(struct snd_pmac *chip)
 	int i, err;
 	struct pmac_daca *mix;
 
-	request_module("i2c-powermac");
+	request_module("%s", "i2c-powermac");
 
 	mix = kzalloc(sizeof(*mix), GFP_KERNEL);
 	if (! mix)
diff --git a/sound/ppc/tumbler.c b/sound/ppc/tumbler.c
index f746e15..efa11f8 100644
--- a/sound/ppc/tumbler.c
+++ b/sound/ppc/tumbler.c
@@ -1345,7 +1345,7 @@ int __init snd_pmac_tumbler_init(struct snd_pmac *chip)
 	struct device_node *tas_node, *np;
 	char *chipname;
 
-	request_module("i2c-powermac");
+	request_module("%s", "i2c-powermac");
 
 	mix = kzalloc(sizeof(*mix), GFP_KERNEL);
 	if (! mix)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  3:35 [PATCH] fix calls to request_module() Nguyen Anh Quynh
@ 2008-12-11  4:01 ` Al Viro
  2008-12-11  4:14   ` Nguyen Anh Quynh
  2008-12-11  4:14   ` Andrew Morton
  0 siblings, 2 replies; 16+ messages in thread
From: Al Viro @ 2008-12-11  4:01 UTC (permalink / raw)
  To: Nguyen Anh Quynh; +Cc: Andrew Morton, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 12:35:21PM +0900, Nguyen Anh Quynh wrote:
> Hi,
> 
> The request_module() function should always have the 1st param as a
> format argument. So for example, request_module("i2c-powermac") should
> be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
> like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
> patch fixes them all in linus-git tree.

... and it doesn't address the underlying problems at all.  A string literal
without a single % in it is a perfectly sane and valid format.  _Why_ are
we getting these warning?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:01 ` Al Viro
@ 2008-12-11  4:14   ` Nguyen Anh Quynh
  2008-12-11  4:14   ` Andrew Morton
  1 sibling, 0 replies; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  4:14 UTC (permalink / raw)
  To: Al Viro; +Cc: Andrew Morton, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 1:01 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Dec 11, 2008 at 12:35:21PM +0900, Nguyen Anh Quynh wrote:
>> Hi,
>>
>> The request_module() function should always have the 1st param as a
>> format argument. So for example, request_module("i2c-powermac") should
>> be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
>> like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
>> patch fixes them all in linus-git tree.
>
> ... and it doesn't address the underlying problems at all.  A string literal
> without a single % in it is a perfectly sane and valid format.  _Why_ are
> we getting these warning?

Hmm, I checked again, and the warnings actually lie elsewhere. I got
mistake while work with several versions at the same time. So please
ignore this patch.

Thanks,
Quynh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:01 ` Al Viro
  2008-12-11  4:14   ` Nguyen Anh Quynh
@ 2008-12-11  4:14   ` Andrew Morton
  2008-12-11  4:23     ` Nguyen Anh Quynh
  2008-12-11  4:49     ` Al Viro
  1 sibling, 2 replies; 16+ messages in thread
From: Andrew Morton @ 2008-12-11  4:14 UTC (permalink / raw)
  To: Al Viro; +Cc: Nguyen Anh Quynh, LKML, Kuniyasu Suzaki

On Thu, 11 Dec 2008 04:01:18 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:

> On Thu, Dec 11, 2008 at 12:35:21PM +0900, Nguyen Anh Quynh wrote:
> > Hi,
> > 
> > The request_module() function should always have the 1st param as a
> > format argument. So for example, request_module("i2c-powermac") should
> > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
> > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
> > patch fixes them all in linus-git tree.
> 
> ... and it doesn't address the underlying problems at all.  A string literal
> without a single % in it is a perfectly sane and valid format.  _Why_ are
> we getting these warning?

extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));

?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:14   ` Andrew Morton
@ 2008-12-11  4:23     ` Nguyen Anh Quynh
  2008-12-11  4:40       ` Jianjun Kong
  2008-12-11  5:00       ` Al Viro
  2008-12-11  4:49     ` Al Viro
  1 sibling, 2 replies; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  4:23 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Al Viro, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 1:14 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 11 Dec 2008 04:01:18 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
>
>> On Thu, Dec 11, 2008 at 12:35:21PM +0900, Nguyen Anh Quynh wrote:
>> > Hi,
>> >
>> > The request_module() function should always have the 1st param as a
>> > format argument. So for example, request_module("i2c-powermac") should
>> > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
>> > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
>> > patch fixes them all in linus-git tree.
>>
>> ... and it doesn't address the underlying problems at all.  A string literal
>> without a single % in it is a perfectly sane and valid format.  _Why_ are
>> we getting these warning?
>
> extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
>

Sorry that after the mail of Viro, I checked again by recover a code
and recompile, but got no warning. But actually that code should not
be compiled at all.

So I checked again by fixing the code that should be compiled
(sound/core/sound.c), and can confirm that without the patch we got
warning like below:

sound/core/sound.c: In function 'snd_request_other':
sound/core/sound.c:91: warning: format not a string literal and no
format arguments

And with the patch, the warnings are all gone.

So Andrew, please consider taking the patch.

Thanks,
Quynh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:23     ` Nguyen Anh Quynh
@ 2008-12-11  4:40       ` Jianjun Kong
  2008-12-11  4:43         ` Nguyen Anh Quynh
  2008-12-11  5:00       ` Al Viro
  1 sibling, 1 reply; 16+ messages in thread
From: Jianjun Kong @ 2008-12-11  4:40 UTC (permalink / raw)
  To: Nguyen Anh Quynh; +Cc: Andrew Morton, Al Viro, LKML, Kuniyasu Suzaki


* Nguyen Anh Quynh wrote:

>On Thu, Dec 11, 2008 at 1:14 PM, Andrew Morton
><akpm@linux-foundation.org> wrote:
>> On Thu, 11 Dec 2008 04:01:18 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:

>>> > The request_module() function should always have the 1st param as a
>>> > format argument. So for example, request_module("i2c-powermac") should
>>> > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
>>> > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
>>> > patch fixes them all in linus-git tree.
>>>
>>> ... and it doesn't address the underlying problems at all.  A string literal
>>> without a single % in it is a perfectly sane and valid format.  _Why_ are
>>> we getting these warning?
>>
>> extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
>>
>
>Sorry that after the mail of Viro, I checked again by recover a code
>and recompile, but got no warning. But actually that code should not
>be compiled at all.
>
>So I checked again by fixing the code that should be compiled
>(sound/core/sound.c), and can confirm that without the patch we got
>warning like below:
>
>sound/core/sound.c: In function 'snd_request_other':
>sound/core/sound.c:91: warning: format not a string literal and no
>format arguments

Hi,all
I also use Ubuntu 8.10, gcc version 4.3.2 (Ubuntu 4.3.2-1ubuntu11)

When I compile the latest kernel, there are more warning. Like this:

scripts/genksyms/lex.c: In function ‘yylex1’:
scripts/genksyms/lex.l:97: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
scripts/mod/modpost.c: In function ‘get_markers’:
scripts/mod/modpost.c:1542: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result
scripts/mod/modpost.c: In function ‘add_marker’:
scripts/mod/modpost.c:1962: warning: ignoring return value of ‘asprintf’, declared with attribute warn_unused_result
scripts/kallsyms.c: In function ‘read_symbol’:
scripts/kallsyms.c:74: warning: ignoring return value of ‘fgets’, declared with attribute warn_unused_result
init/main.c: In function ‘start_kernel’:
init/main.c:571: warning: format not a string literal and no format arguments
init/initramfs.c: In function ‘populate_rootfs’:
init/initramfs.c:581: warning: format not a string literal and no format arguments
usr/gen_init_cpio.c: In function ‘cpio_mkfile’:
usr/gen_init_cpio.c:357: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
arch/x86/kernel/dumpstack_32.c: In function ‘print_trace_warning_symbol’:
arch/x86/kernel/dumpstack_32.c:125: warning: format not a string literal and no format arguments
arch/x86/kernel/dumpstack_32.c: In function ‘print_trace_address’:
arch/x86/kernel/dumpstack_32.c:147: warning: format not a string literal and no format arguments
arch/x86/kernel/e820.c: In function ‘early_panic’:
arch/x86/kernel/e820.c:1172: warning: format not a string literal and no format arguments
arch/x86/kernel/e820.c:1173: warning: format not a string literal and no format arguments
kernel/power/main.c: In function ‘test_suspend’:
kernel/power/main.c:720: warning: format not a string literal and no format arguments
fs/dquot.c: In function ‘find_quota_format’:
fs/dquot.c:170: warning: format not a string literal and no format arguments
crypto/api.c: In function ‘crypto_larval_lookup’:
crypto/api.c:218: warning: format not a string literal and no format arguments
crypto/algapi.c: In function ‘crypto_lookup_template’:
crypto/algapi.c:427: warning: format not a string literal and no format arguments
...
...

-- 
Jianjun Kong |Happy Hacking
Homepage: http://kongove.cn
Gtalk:KongJianjun@gmail.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:40       ` Jianjun Kong
@ 2008-12-11  4:43         ` Nguyen Anh Quynh
  2008-12-11  5:02           ` Jianjun Kong
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  4:43 UTC (permalink / raw)
  To: Jianjun Kong; +Cc: Andrew Morton, Al Viro, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 1:40 PM, Jianjun Kong <jianjun@zeuux.org> wrote:
>
> * Nguyen Anh Quynh wrote:
>
>>On Thu, Dec 11, 2008 at 1:14 PM, Andrew Morton
>><akpm@linux-foundation.org> wrote:
>>> On Thu, 11 Dec 2008 04:01:18 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
>
>>>> > The request_module() function should always have the 1st param as a
>>>> > format argument. So for example, request_module("i2c-powermac") should
>>>> > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
>>>> > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
>>>> > patch fixes them all in linus-git tree.
>>>>
>>>> ... and it doesn't address the underlying problems at all.  A string literal
>>>> without a single % in it is a perfectly sane and valid format.  _Why_ are
>>>> we getting these warning?
>>>
>>> extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
>>>
>>
>>Sorry that after the mail of Viro, I checked again by recover a code
>>and recompile, but got no warning. But actually that code should not
>>be compiled at all.
>>
>>So I checked again by fixing the code that should be compiled
>>(sound/core/sound.c), and can confirm that without the patch we got
>>warning like below:
>>
>>sound/core/sound.c: In function 'snd_request_other':
>>sound/core/sound.c:91: warning: format not a string literal and no
>>format arguments
>
> Hi,all
> I also use Ubuntu 8.10, gcc version 4.3.2 (Ubuntu 4.3.2-1ubuntu11)
>
> When I compile the latest kernel, there are more warning. Like this:
>
> scripts/genksyms/lex.c: In function 'yylex1':
> scripts/genksyms/lex.l:97: warning: ignoring return value of 'fwrite', declared with attribute warn_unused_result
> scripts/mod/modpost.c: In function 'get_markers':
> scripts/mod/modpost.c:1542: warning: ignoring return value of 'asprintf', declared with attribute warn_unused_result
> scripts/mod/modpost.c: In function 'add_marker':
> scripts/mod/modpost.c:1962: warning: ignoring return value of 'asprintf', declared with attribute warn_unused_result
> scripts/kallsyms.c: In function 'read_symbol':
> scripts/kallsyms.c:74: warning: ignoring return value of 'fgets', declared with attribute warn_unused_result
> init/main.c: In function 'start_kernel':
> init/main.c:571: warning: format not a string literal and no format arguments
> init/initramfs.c: In function 'populate_rootfs':
> init/initramfs.c:581: warning: format not a string literal and no format arguments
> usr/gen_init_cpio.c: In function 'cpio_mkfile':
> usr/gen_init_cpio.c:357: warning: ignoring return value of 'fwrite', declared with attribute warn_unused_result
> arch/x86/kernel/dumpstack_32.c: In function 'print_trace_warning_symbol':
> arch/x86/kernel/dumpstack_32.c:125: warning: format not a string literal and no format arguments
> arch/x86/kernel/dumpstack_32.c: In function 'print_trace_address':
> arch/x86/kernel/dumpstack_32.c:147: warning: format not a string literal and no format arguments
> arch/x86/kernel/e820.c: In function 'early_panic':
> arch/x86/kernel/e820.c:1172: warning: format not a string literal and no format arguments
> arch/x86/kernel/e820.c:1173: warning: format not a string literal and no format arguments
> kernel/power/main.c: In function 'test_suspend':
> kernel/power/main.c:720: warning: format not a string literal and no format arguments
> fs/dquot.c: In function 'find_quota_format':
> fs/dquot.c:170: warning: format not a string literal and no format arguments
> crypto/api.c: In function 'crypto_larval_lookup':
> crypto/api.c:218: warning: format not a string literal and no format arguments
> crypto/algapi.c: In function 'crypto_lookup_template':
> crypto/algapi.c:427: warning: format not a string literal and no format arguments
> ...

I saw the same thing. My patch only fixed part of them.

So please submit your patch to fix the remainig :-)

Q

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:14   ` Andrew Morton
  2008-12-11  4:23     ` Nguyen Anh Quynh
@ 2008-12-11  4:49     ` Al Viro
  2008-12-11  5:03       ` Roland Dreier
  1 sibling, 1 reply; 16+ messages in thread
From: Al Viro @ 2008-12-11  4:49 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Nguyen Anh Quynh, LKML, Kuniyasu Suzaki

On Wed, Dec 10, 2008 at 08:14:55PM -0800, Andrew Morton wrote:
> On Thu, 11 Dec 2008 04:01:18 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:
> 
> > On Thu, Dec 11, 2008 at 12:35:21PM +0900, Nguyen Anh Quynh wrote:
> > > Hi,
> > > 
> > > The request_module() function should always have the 1st param as a
> > > format argument. So for example, request_module("i2c-powermac") should
> > > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
> > > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
> > > patch fixes them all in linus-git tree.
> > 
> > ... and it doesn't address the underlying problems at all.  A string literal
> > without a single % in it is a perfectly sane and valid format.  _Why_ are
> > we getting these warning?
> 
> extern int request_module(const char * name, ...) __attribute__ ((format (printf, 1, 2)));
> 
> ?

... and request_module("i2c-powermac") should be perfectly valid, shouldn't it?
I mean, I do not believe that any gcc version would start spewing warnings
of
	printf("-- \n");
and its ilk...

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:23     ` Nguyen Anh Quynh
  2008-12-11  4:40       ` Jianjun Kong
@ 2008-12-11  5:00       ` Al Viro
  2008-12-11  5:42         ` Nguyen Anh Quynh
  1 sibling, 1 reply; 16+ messages in thread
From: Al Viro @ 2008-12-11  5:00 UTC (permalink / raw)
  To: Nguyen Anh Quynh; +Cc: Andrew Morton, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 01:23:36PM +0900, Nguyen Anh Quynh wrote:

> So I checked again by fixing the code that should be compiled
> (sound/core/sound.c), and can confirm that without the patch we got
> warning like below:
> 
> sound/core/sound.c: In function 'snd_request_other':
> sound/core/sound.c:91: warning: format not a string literal and no
> format arguments

Ah, but that's different.  Take a look at that warning and think _why_
it is given and what is it about.  Getting an untrusted string as
format argument is a real security hole, but it has nothing to do
with a pile of cases in your patch.

FWIW, even in this case (where the warning is a false positive, BTW -
the string passed there can have one of two values and both are safe)
I would simply do
	switch(...) {
	case .... : request_module("snd-seq"); break;
	case .... : request_module("snd-timer"); break;
	}
and that's it.

Slapping "%s" here actually obfuscates the code, without making it safer.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:43         ` Nguyen Anh Quynh
@ 2008-12-11  5:02           ` Jianjun Kong
  0 siblings, 0 replies; 16+ messages in thread
From: Jianjun Kong @ 2008-12-11  5:02 UTC (permalink / raw)
  To: Nguyen Anh Quynh; +Cc: Andrew Morton, Al Viro, LKML, Kuniyasu Suzaki


* Nguyen Anh Quynh wrote:

>On Thu, Dec 11, 2008 at 1:40 PM, Jianjun Kong <jianjun@zeuux.org> wrote:
>> * Nguyen Anh Quynh wrote:
>>>>> > The request_module() function should always have the 1st param as a
>>>>> > format argument. So for example, request_module("i2c-powermac") should
>>>>> > be called as request_module("%s", "i2c-powermac"). Otherwise, new gcc
>>>>> > like gcc 4.3.2 on Ubuntu 8.10 would spit out a lot of warnings. This
>>>>> > patch fixes them all in linus-git tree.

<sign>
>> Hi,all
>> I also use Ubuntu 8.10, gcc version 4.3.2 (Ubuntu 4.3.2-1ubuntu11)
>>
>> When I compile the latest kernel, there are more warning. Like this:

<sign>
>> arch/x86/kernel/e820.c: In function 'early_panic':
>> arch/x86/kernel/e820.c:1172: warning: format not a string literal and no format arguments
>> arch/x86/kernel/e820.c:1173: warning: format not a string literal and no format arguments
>> kernel/power/main.c: In function 'test_suspend':
>> kernel/power/main.c:720: warning: format not a string literal and no format arguments
>> fs/dquot.c: In function 'find_quota_format':
>> fs/dquot.c:170: warning: format not a string literal and no format arguments
>> crypto/api.c: In function 'crypto_larval_lookup':
>> crypto/api.c:218: warning: format not a string literal and no format arguments
>> crypto/algapi.c: In function 'crypto_lookup_template':
>> crypto/algapi.c:427: warning: format not a string literal and no format arguments
>> ...
>
>I saw the same thing. My patch only fixed part of them.
>
>So please submit your patch to fix the remainig :-)

But there is no this kind of warns at Fedora 10, gcc version 4.3.2 20081105 (Red Hat 4.3.2-7)

We should make clean whether your patch is correct first. 
There are many more that kind of warns. I just list little of them.


	Jianjun
-- 
Jianjun Kong |Happy Hacking
Homepage: http://kongove.cn
Gtalk:KongJianjun@gmail.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  4:49     ` Al Viro
@ 2008-12-11  5:03       ` Roland Dreier
  2008-12-11  5:41         ` Andrew Morton
  0 siblings, 1 reply; 16+ messages in thread
From: Roland Dreier @ 2008-12-11  5:03 UTC (permalink / raw)
  To: Al Viro; +Cc: Andrew Morton, Nguyen Anh Quynh, LKML, Kuniyasu Suzaki

 > I mean, I do not believe that any gcc version would start spewing warnings
 > of
 > 	printf("-- \n");
 > and its ilk...

No, I haven't seen gcc warn about anything that crazy (ie where it can
see the format string and prove it's safe).

I do see warnings from the Ubuntu gcc with code like:

#include <stdio.h>

extern char *a;

void foo()
{
	printf(a);
}

which produces

a.c: In function 'foo':
a.c:7: warning: format not a string literal and no format arguments


The kernel has such code eg in init/main.c, which does

	printk(linux_banner);

when linux_banner is only visible to the compiler as

extern const char linux_banner[];

however the trivial fix

diff --git a/init/main.c b/init/main.c
index 7e117a2..e471598 100644
--- a/init/main.c
+++ b/init/main.c
@@ -568,7 +568,7 @@ asmlinkage void __init start_kernel(void)
 	boot_cpu_init();
 	page_address_init();
 	printk(KERN_NOTICE);
-	printk(linux_banner);
+	printk("%s", linux_banner);
 	setup_arch(&command_line);
 	mm_init_owner(&init_mm, &init_task);
 	setup_command_line(command_line);

doesn't seem that appealing, since it bloats the object code for a
non-bug -- 7 bytes for me on x86_64:

add/remove: 0/0 grow/shrink: 1/0 up/down: 7/0 (7)
function                                     old     new   delta
start_kernel                                 680     687      +7

given the number of such warnings I see in a typical compile, this would
be a fairly hefty amount of bloat just to shut up gcc.

On the other hand, gcc warning on such code (untrusted format string
passed into a printf-like function) seems quite legitimate as well.

So I dunno.

 - Roland

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  5:03       ` Roland Dreier
@ 2008-12-11  5:41         ` Andrew Morton
  2008-12-11 23:25           ` Roland Dreier
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2008-12-11  5:41 UTC (permalink / raw)
  To: Roland Dreier; +Cc: Al Viro, Nguyen Anh Quynh, LKML, Kuniyasu Suzaki

On Wed, 10 Dec 2008 21:03:37 -0800 Roland Dreier <rdreier@cisco.com> wrote:

> The kernel has such code eg in init/main.c, which does
> 
> 	printk(linux_banner);
> 
> when linux_banner is only visible to the compiler as
> 
> extern const char linux_banner[];
> 
> however the trivial fix
> 
> diff --git a/init/main.c b/init/main.c
> index 7e117a2..e471598 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -568,7 +568,7 @@ asmlinkage void __init start_kernel(void)
>  	boot_cpu_init();
>  	page_address_init();
>  	printk(KERN_NOTICE);
> -	printk(linux_banner);
> +	printk("%s", linux_banner);
>  	setup_arch(&command_line);
>  	mm_init_owner(&init_mm, &init_task);
>  	setup_command_line(command_line);
> 
> doesn't seem that appealing, since it bloats the object code for a
> non-bug -- 7 bytes for me on x86_64:
> 
> add/remove: 0/0 grow/shrink: 1/0 up/down: 7/0 (7)
> function                                     old     new   delta
> start_kernel                                 680     687      +7
> 
> given the number of such warnings I see in a typical compile, this would
> be a fairly hefty amount of bloat just to shut up gcc.

yes, that would suck.  otoh, our current warning spew actually causes bugs.

I wonder if we could add a printk_stfu() which isn't declared
attribute(printf) and which simply calls printk.  We might still get a
single warning at the interface point.

> On the other hand, gcc warning on such code (untrusted format string
> passed into a printf-like function) seems quite legitimate as well.

Yes, we've had actual bugs in the kernel from this, where the control
string was user-provided.  root-only user, fortunately.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  5:00       ` Al Viro
@ 2008-12-11  5:42         ` Nguyen Anh Quynh
  2008-12-11  6:00           ` Al Viro
  0 siblings, 1 reply; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  5:42 UTC (permalink / raw)
  To: Al Viro; +Cc: Andrew Morton, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 2:00 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Dec 11, 2008 at 01:23:36PM +0900, Nguyen Anh Quynh wrote:
>
>> So I checked again by fixing the code that should be compiled
>> (sound/core/sound.c), and can confirm that without the patch we got
>> warning like below:
>>
>> sound/core/sound.c: In function 'snd_request_other':
>> sound/core/sound.c:91: warning: format not a string literal and no
>> format arguments
>
> Ah, but that's different.  Take a look at that warning and think _why_
> it is given and what is it about.  Getting an untrusted string as
> format argument is a real security hole, but it has nothing to do
> with a pile of cases in your patch.

Yes, clearly the warning is to warn us about potential format string
bugs. But I agree that there are a lot of false possitives.

My patch is mainly to make gcc happy.

Thanks,
Q

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  5:42         ` Nguyen Anh Quynh
@ 2008-12-11  6:00           ` Al Viro
  2008-12-11  6:28             ` Nguyen Anh Quynh
  0 siblings, 1 reply; 16+ messages in thread
From: Al Viro @ 2008-12-11  6:00 UTC (permalink / raw)
  To: Nguyen Anh Quynh; +Cc: Andrew Morton, LKML, Kuniyasu Suzaki

On Thu, Dec 11, 2008 at 02:42:28PM +0900, Nguyen Anh Quynh wrote:
> > Ah, but that's different.  Take a look at that warning and think _why_
> > it is given and what is it about.  Getting an untrusted string as
> > format argument is a real security hole, but it has nothing to do
> > with a pile of cases in your patch.
> 
> Yes, clearly the warning is to warn us about potential format string
> bugs. But I agree that there are a lot of false possitives.
> 
> My patch is mainly to make gcc happy.

Your patch is mostly obfuscating the places gcc does *not* warn about...

Looking through it, only
drivers/media/video/cx18/cx18-driver.c
drivers/media/video/ivtv/ivtv-driver.c
drivers/media/video/pvrusb2/pvrusb2-hdw.c
drivers/media/video/zoran/zoran_card.c
drivers/mtd/chips/gen_probe.c
drivers/net/wireless/hostap/hostap_ioctl.c
drivers/of/of_spi.c
drivers/usb/storage/libusual.c
fs/dquot.c
fs/gfs2/locking.c
net/ieee80211/ieee80211_wx.c
sound/core/sound.c

are not of <string literal> -> "%s", <string literal> variety.  Everything
else has never generated a format warning.  At least trim the patch,
removing the obviously useless parts.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  6:00           ` Al Viro
@ 2008-12-11  6:28             ` Nguyen Anh Quynh
  0 siblings, 0 replies; 16+ messages in thread
From: Nguyen Anh Quynh @ 2008-12-11  6:28 UTC (permalink / raw)
  To: Al Viro, Andrew Morton; +Cc: LKML, Kuniyasu Suzaki

[-- Attachment #1: Type: text/plain, Size: 2212 bytes --]

On Thu, Dec 11, 2008 at 3:00 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Dec 11, 2008 at 02:42:28PM +0900, Nguyen Anh Quynh wrote:
>> > Ah, but that's different.  Take a look at that warning and think _why_
>> > it is given and what is it about.  Getting an untrusted string as
>> > format argument is a real security hole, but it has nothing to do
>> > with a pile of cases in your patch.
>>
>> Yes, clearly the warning is to warn us about potential format string
>> bugs. But I agree that there are a lot of false possitives.
>>
>> My patch is mainly to make gcc happy.
>
> Your patch is mostly obfuscating the places gcc does *not* warn about...
>
> Looking through it, only
> drivers/media/video/cx18/cx18-driver.c
> drivers/media/video/ivtv/ivtv-driver.c
> drivers/media/video/pvrusb2/pvrusb2-hdw.c
> drivers/media/video/zoran/zoran_card.c
> drivers/mtd/chips/gen_probe.c
> drivers/net/wireless/hostap/hostap_ioctl.c
> drivers/of/of_spi.c
> drivers/usb/storage/libusual.c
> fs/dquot.c
> fs/gfs2/locking.c
> net/ieee80211/ieee80211_wx.c
> sound/core/sound.c
>
> are not of <string literal> -> "%s", <string literal> variety.  Everything
> else has never generated a format warning.  At least trim the patch,
> removing the obviously useless parts.

Yes, initially I misunderstood these warnings.

So here is the patch, again, with only the "correct" warning fixed.

Signed-off-by: Nguyen Anh Quynh <aquynh@gmail.com>

Thanks,
Quynh


# diffstat fix-request_module-call.linus_git_tree.patch
 drivers/media/video/cx18/cx18-driver.c     |    2 +-
 drivers/media/video/ivtv/ivtv-driver.c     |    2 +-
 drivers/media/video/pvrusb2/pvrusb2-hdw.c  |    2 +-
 drivers/media/video/zoran/zoran_card.c     |    8 ++++----
 drivers/mtd/chips/gen_probe.c              |    2 +-
 drivers/net/wireless/hostap/hostap_ioctl.c |    2 +-
 drivers/of/of_spi.c                        |    2 +-
 drivers/usb/storage/libusual.c             |    2 +-
 fs/dquot.c                                 |    2 +-
 fs/gfs2/locking.c                          |    2 +-
 net/ieee80211/ieee80211_wx.c               |    2 +-
 sound/core/sound.c                         |    2 +-
 12 files changed, 15 insertions(+), 15 deletions(-)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix-request_module-call.linus_git_tree.patch --]
[-- Type: text/x-patch; name=fix-request_module-call.linus_git_tree.patch, Size: 6675 bytes --]

diff --git a/drivers/media/video/cx18/cx18-driver.c b/drivers/media/video/cx18/cx18-driver.c
index 7874d97..81f0d5e 100644
--- a/drivers/media/video/cx18/cx18-driver.c
+++ b/drivers/media/video/cx18/cx18-driver.c
@@ -572,7 +572,7 @@ static u32 cx18_request_module(struct cx18 *cx, u32 hw,
 {
 	if ((hw & id) == 0)
 		return hw;
-	if (request_module(name) != 0) {
+	if (request_module("%s", name) != 0) {
 		CX18_ERR("Failed to load module %s\n", name);
 		return hw & ~id;
 	}
diff --git a/drivers/media/video/ivtv/ivtv-driver.c b/drivers/media/video/ivtv/ivtv-driver.c
index b69cc1d..a26ad59 100644
--- a/drivers/media/video/ivtv/ivtv-driver.c
+++ b/drivers/media/video/ivtv/ivtv-driver.c
@@ -859,7 +859,7 @@ static u32 ivtv_request_module(struct ivtv *itv, u32 hw,
 {
 	if ((hw & id) == 0)
 		return hw;
-	if (request_module(name) != 0) {
+	if (request_module("%s", name) != 0) {
 		IVTV_ERR("Failed to load module %s\n", name);
 		return hw & ~id;
 	}
diff --git a/drivers/media/video/pvrusb2/pvrusb2-hdw.c b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
index 5b81ba4..6d75d3c 100644
--- a/drivers/media/video/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/video/pvrusb2/pvrusb2-hdw.c
@@ -1967,7 +1967,7 @@ static void pvr2_hdw_setup_low(struct pvr2_hdw *hdw)
 	if (!pvr2_hdw_dev_ok(hdw)) return;
 
 	for (idx = 0; idx < hdw->hdw_desc->client_modules.cnt; idx++) {
-		request_module(hdw->hdw_desc->client_modules.lst[idx]);
+		request_module("%s", hdw->hdw_desc->client_modules.lst[idx]);
 	}
 
 	if (!hdw->hdw_desc->flag_no_powerup) {
diff --git a/drivers/media/video/zoran/zoran_card.c b/drivers/media/video/zoran/zoran_card.c
index fa5f2f8..586ee7d 100644
--- a/drivers/media/video/zoran/zoran_card.c
+++ b/drivers/media/video/zoran/zoran_card.c
@@ -1435,7 +1435,7 @@ find_zr36057 (void)
 		}
 
 		if (i2c_dec_name) {
-			if ((result = request_module(i2c_dec_name)) < 0) {
+			if ((result = request_module("%s", i2c_dec_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load module %s: %d\n",
@@ -1455,7 +1455,7 @@ find_zr36057 (void)
 		}
 
 		if (i2c_enc_name) {
-			if ((result = request_module(i2c_enc_name)) < 0) {
+			if ((result = request_module("%s", i2c_enc_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load module %s: %d\n",
@@ -1478,7 +1478,7 @@ find_zr36057 (void)
 		if (zr->card.video_codec != 0 &&
 		    (codec_name =
 		     codecid_to_modulename(zr->card.video_codec)) != NULL) {
-			if ((result = request_module(codec_name)) < 0) {
+			if ((result = request_module("%s", codec_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load modules %s: %d\n",
@@ -1488,7 +1488,7 @@ find_zr36057 (void)
 		if (zr->card.video_vfe != 0 &&
 		    (vfe_name =
 		     codecid_to_modulename(zr->card.video_vfe)) != NULL) {
-			if ((result = request_module(vfe_name)) < 0) {
+			if ((result = request_module("%s", vfe_name)) < 0) {
 				dprintk(1,
 					KERN_ERR
 					"%s: failed to load modules %s: %d\n",
diff --git a/drivers/mtd/chips/gen_probe.c b/drivers/mtd/chips/gen_probe.c
index e2dc964..814d4f3 100644
--- a/drivers/mtd/chips/gen_probe.c
+++ b/drivers/mtd/chips/gen_probe.c
@@ -212,7 +212,7 @@ static inline struct mtd_info *cfi_cmdset_unknown(struct map_info *map,
 
 	probe_function = __symbol_get(probename);
 	if (!probe_function) {
-		request_module(probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
+		request_module("%s", probename + sizeof(MODULE_SYMBOL_PREFIX) - 1);
 		probe_function = __symbol_get(probename);
 	}
 
diff --git a/drivers/net/wireless/hostap/hostap_ioctl.c b/drivers/net/wireless/hostap/hostap_ioctl.c
index 3f8b1d7..ea03992 100644
--- a/drivers/net/wireless/hostap/hostap_ioctl.c
+++ b/drivers/net/wireless/hostap/hostap_ioctl.c
@@ -3297,7 +3297,7 @@ static int prism2_ioctl_siwencodeext(struct net_device *dev,
 
 	ops = ieee80211_get_crypto_ops(alg);
 	if (ops == NULL) {
-		request_module(module);
+		request_module("%s", module);
 		ops = ieee80211_get_crypto_ops(alg);
 	}
 	if (ops == NULL) {
diff --git a/drivers/of/of_spi.c b/drivers/of/of_spi.c
index bed0ed6..ad96437 100644
--- a/drivers/of/of_spi.c
+++ b/drivers/of/of_spi.c
@@ -82,7 +82,7 @@ void of_register_spi_devices(struct spi_master *master, struct device_node *np)
 		spi->dev.archdata.of_node = nc;
 
 		/* Register the new device */
-		request_module(spi->modalias);
+		request_module("%s", spi->modalias);
 		rc = spi_add_device(spi);
 		if (rc) {
 			dev_err(&master->dev, "spi_device register error %s\n",
diff --git a/drivers/usb/storage/libusual.c b/drivers/usb/storage/libusual.c
index d617e8a..a481e02 100644
--- a/drivers/usb/storage/libusual.c
+++ b/drivers/usb/storage/libusual.c
@@ -180,7 +180,7 @@ static int usu_probe_thread(void *arg)
 	unsigned long flags;
 
 	mutex_lock(&usu_probe_mutex);
-	rc = request_module(bias_names[type]);
+	rc = request_module("%s", bias_names[type]);
 	spin_lock_irqsave(&usu_lock, flags);
 	if (rc == 0 && (st->fls & USU_MOD_FL_PRESENT) == 0) {
 		/*
diff --git a/fs/dquot.c b/fs/dquot.c
index 5e95261..3984307 100644
--- a/fs/dquot.c
+++ b/fs/dquot.c
@@ -167,7 +167,7 @@ static struct quota_format_type *find_quota_format(int id)
 		spin_unlock(&dq_list_lock);
 		
 		for (qm = 0; module_names[qm].qm_fmt_id && module_names[qm].qm_fmt_id != id; qm++);
-		if (!module_names[qm].qm_fmt_id || request_module(module_names[qm].qm_mod_name))
+		if (!module_names[qm].qm_fmt_id || request_module("%s", module_names[qm].qm_mod_name))
 			return NULL;
 
 		spin_lock(&dq_list_lock);
diff --git a/fs/gfs2/locking.c b/fs/gfs2/locking.c
index 523243a..7a9df7f 100644
--- a/fs/gfs2/locking.c
+++ b/fs/gfs2/locking.c
@@ -177,7 +177,7 @@ retry:
 		if (!try && capable(CAP_SYS_MODULE)) {
 			try = 1;
 			mutex_unlock(&lmh_lock);
-			request_module(proto_name);
+			request_module("%s", proto_name);
 			goto retry;
 		}
 		printk(KERN_INFO "GFS2: can't find protocol %s\n", proto_name);
diff --git a/net/ieee80211/ieee80211_wx.c b/net/ieee80211/ieee80211_wx.c
index 973832d..de4be03 100644
--- a/net/ieee80211/ieee80211_wx.c
+++ b/net/ieee80211/ieee80211_wx.c
@@ -608,7 +608,7 @@ int ieee80211_wx_set_encodeext(struct ieee80211_device *ieee,
 
 	ops = ieee80211_get_crypto_ops(alg);
 	if (ops == NULL) {
-		request_module(module);
+		request_module("%s", module);
 		ops = ieee80211_get_crypto_ops(alg);
 	}
 	if (ops == NULL) {
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 44a69bb..c143b9c 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -88,7 +88,7 @@ static void snd_request_other(int minor)
 	case SNDRV_MINOR_TIMER:		str = "snd-timer";	break;
 	default:			return;
 	}
-	request_module(str);
+	request_module("%s", str);
 }
 
 #endif	/* modular kernel */

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH] fix calls to request_module()
  2008-12-11  5:41         ` Andrew Morton
@ 2008-12-11 23:25           ` Roland Dreier
  0 siblings, 0 replies; 16+ messages in thread
From: Roland Dreier @ 2008-12-11 23:25 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Al Viro, Nguyen Anh Quynh, LKML, Kuniyasu Suzaki

 > I wonder if we could add a printk_stfu() which isn't declared
 > attribute(printf) and which simply calls printk.  We might still get a
 > single warning at the interface point.

That would fix the class of warnings like

	printk(char_pointer_gcc_doesnt_know_is_safe);

but therre are alsofunctions like device_create() etc etc where someone
could easily and legitimately do the same thing.

 - R.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2008-12-11 23:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-12-11  3:35 [PATCH] fix calls to request_module() Nguyen Anh Quynh
2008-12-11  4:01 ` Al Viro
2008-12-11  4:14   ` Nguyen Anh Quynh
2008-12-11  4:14   ` Andrew Morton
2008-12-11  4:23     ` Nguyen Anh Quynh
2008-12-11  4:40       ` Jianjun Kong
2008-12-11  4:43         ` Nguyen Anh Quynh
2008-12-11  5:02           ` Jianjun Kong
2008-12-11  5:00       ` Al Viro
2008-12-11  5:42         ` Nguyen Anh Quynh
2008-12-11  6:00           ` Al Viro
2008-12-11  6:28             ` Nguyen Anh Quynh
2008-12-11  4:49     ` Al Viro
2008-12-11  5:03       ` Roland Dreier
2008-12-11  5:41         ` Andrew Morton
2008-12-11 23:25           ` Roland Dreier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox