I decided to ruin my Sunday with an utterly pointless activity --
waging war on -mm build warnings. Some of the code I touched belonged
to grotty, unused, dying drivers, but still, the end result was that
I can now only see 5 warnings remaining on my typical .config (those
have to do with pci_{find,enable}_device deprecation and __must_check
on sysfs_create_{file,link} so I'll just leave 'em for now).
Some were unused variable warnings, some uninitialized (few of those
real bugs, but most not), some were "function defined but not used",
some function argument signedness mismatches, some __must_check fixes,
some tricky, but most just trivial.
So till the next kernel release adds back shiny new build warnings,
here's to some temporary peace ...
drivers/usb/misc/sisusbvga/sisusb.c: In function $B!F(Jusb_sisusb_init$B!G(J:
drivers/usb/misc/sisusbvga/sisusb.c:3321: warning: unused variable $B!F(Jsisusb$B!G(J
drivers/usb/misc/sisusbvga/sisusb.c:3320: warning: unused variable $B!F(Jretval$B!G(J
are trivially solved by getting rid of the unused variables.
drivers/usb/misc/sisusbvga/sisusb.c: In function $B!F(Jsisusb_open$B!G(J:
drivers/usb/misc/sisusbvga/sisusb.c:2444: warning: $B!F(Jsisusb$B!G(J is used uninitialized in this function
is a genuine bug (which will cause oops). We cannot use "sisusb" in
error path for (!interface), because sisusb will itself be derived
from "interface" later.
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/usb/misc/sisusbvga/sisusb.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
--- linux-2.6.23-rc4-mm1/drivers/usb/misc/sisusbvga/sisusb.c~fix 2007-09-02 19:06:01.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/usb/misc/sisusbvga/sisusb.c 2007-09-02 19:10:52.000000000 +0530
@@ -2440,10 +2440,8 @@ sisusb_open(struct inode *inode, struct
struct usb_interface *interface;
int subminor = iminor(inode);
- if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
- dev_err(&sisusb->sisusb_dev->dev, "Failed to find interface\n");
+ if (!(interface = usb_find_interface(&sisusb_driver, subminor)))
return -ENODEV;
- }
if (!(sisusb = usb_get_intfdata(interface)))
return -ENODEV;
@@ -3317,9 +3315,6 @@ static struct usb_driver sisusb_driver =
static int __init usb_sisusb_init(void)
{
- int retval;
- struct sisusb_usb_data *sisusb;
-
#ifdef INCL_SISUSB_CON
sisusb_init_concode();
#endif
net/sunrpc/svc.c: In function $B!F(J__svc_create_thread$B!G(J:
net/sunrpc/svc.c:550: warning: $B!F(Joldmask.bits[0u]$B!G(J may be used uninitialized in this function
is a bogus warning, but gcc isn't smart enough to see why. We cannot just
reorganize the code in the function, because we want the set_cpus_allowed()
restore to happen only after the kernel_thread() is forked. Alas, we have
to use cpus_clear() to initialize oldmask instead to keep gcc happy.
Also add some comments to describe what's happening in the function.
Signed-off-by: Satyam Sharma <[email protected]>
---
net/sunrpc/svc.c | 7 +++++++
1 file changed, 7 insertions(+)
--- linux-2.6.23-rc4-mm1/net/sunrpc/svc.c~fix 2007-09-02 22:55:25.000000000 +0530
+++ linux-2.6.23-rc4-mm1/net/sunrpc/svc.c 2007-09-02 23:03:45.000000000 +0530
@@ -568,17 +568,24 @@ __svc_create_thread(svc_thread_fn func,
rqstp->rq_server = serv;
rqstp->rq_pool = pool;
+ /* Only to prevent gcc warning */
+ cpus_clear(oldmask);
+
+ /* Temporarily change CPU affinity before forking svc thread */
if (serv->sv_nrpools > 1)
have_oldmask = svc_pool_map_set_cpumask(pool->sp_id, &oldmask);
+ /* Will inherit current->cpus_allowed */
error = kernel_thread((int (*)(void *)) func, rqstp, 0);
+ /* Restore old cpus_allowed */
if (have_oldmask)
set_cpus_allowed(current, oldmask);
if (error < 0)
goto out_thread;
svc_sock_update_bufs(serv);
+
error = 0;
out:
return error;
net/wireless/sysfs.c:108: warning: $B!F(Jwiphy_uevent$B!G(J defined but not used
when CONFIG_HOTPLUG=n is because the only usage site of this function
is #ifdef'ed as such, so let's #ifdef the definition also.
Signed-off-by: Satyam Sharma <[email protected]>
---
net/wireless/sysfs.c | 2 ++
1 file changed, 2 insertions(+)
--- linux-2.6.23-rc4-mm1/net/wireless/sysfs.c~fix 2007-09-02 20:06:11.000000000 +0530
+++ linux-2.6.23-rc4-mm1/net/wireless/sysfs.c 2007-09-02 20:07:01.000000000 +0530
@@ -104,11 +104,13 @@ static void wiphy_dev_release(struct dev
cfg80211_dev_free(rdev);
}
+#ifdef CONFIG_HOTPLUG
static int wiphy_uevent(struct device *dev, struct kobj_uevent_env *env)
{
/* TODO, we probably need stuff here */
return 0;
}
+#endif
struct class ieee80211_class = {
.name = "ieee80211",
drivers/usb/serial/bus.c: In function $B!F(Jusb_serial_bus_deregister$B!G(J:
drivers/usb/serial/bus.c:185:
warning: passing argument 1 of $B!F(Jfree_dynids$B!G(J from incompatible pointer type
Above build warning comes when CONFIG_HOTPLUG=n because argument of
free_dynids() in serial/bus.c is a struct usb_serial_driver, not a
struct usb_driver. This is not a runtime bug, because the function
is an empty stub and never dereferences the passed pointer anyway.
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/usb/serial/bus.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.23-rc4-mm1/drivers/usb/serial/bus.c~fix 2007-09-02 20:33:18.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/usb/serial/bus.c 2007-09-02 20:39:06.000000000 +0530
@@ -154,7 +154,7 @@ static void free_dynids(struct usb_seria
static struct driver_attribute drv_attrs[] = {
__ATTR_NULL,
};
-static inline void free_dynids(struct usb_driver *drv)
+static inline void free_dynids(struct usb_serial_driver *drv)
{
}
#endif
drivers/scsi/ips.c: In function $B!F(Jips_register_scsi$B!G(J:
drivers/scsi/ips.c:6869:
warning: ignoring return value of $B!F(Jscsi_add_host$B!G(J, declared with attribute warn_unused_result
scsi_add_host() is __must_check, so let's check it's return and cleanup
appropriately on errors.
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/scsi/ips.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
--- linux-2.6.23-rc4-mm1/drivers/scsi/ips.c~fix 2007-09-02 20:21:27.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/scsi/ips.c 2007-09-02 20:25:58.000000000 +0530
@@ -6866,7 +6866,12 @@ ips_register_scsi(int index)
sh->max_channel = ha->nbus - 1;
sh->can_queue = ha->max_cmds - 1;
- scsi_add_host(sh, NULL);
+ if (scsi_add_host(sh, NULL)) {
+ IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to add SCSI host\n");
+ free_irq(ha->irq, ha);
+ scsi_host_put(sh);
+ return -1;
+ }
scsi_scan_host(sh);
return 0;
sound/isa/sb16/sb16.c: In function $B!F(Jsnd_sb16_isa_probe$B!G(J:
sound/isa/sb16/sb16.c:559: warning: $B!F(Jerr$B!G(J may be used uninitialized in this function
is a bogus warning, so let's shut it up.
Signed-off-by: Satyam Sharma <[email protected]>
---
sound/isa/sb/sb16.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-2.6.23-rc4-mm1/sound/isa/sb/sb16.c~fix 2007-09-02 21:41:51.000000000 +0530
+++ linux-2.6.23-rc4-mm1/sound/isa/sb/sb16.c 2007-09-02 21:42:56.000000000 +0530
@@ -556,7 +556,6 @@ static int __devinit snd_sb16_isa_match(
static int __devinit snd_sb16_isa_probe(struct device *pdev, unsigned int dev)
{
- int err;
static int possible_irqs[] = {5, 9, 10, 7, -1};
static int possible_dmas8[] = {1, 3, 0, -1};
static int possible_dmas16[] = {5, 6, 7, -1};
@@ -585,6 +584,8 @@ static int __devinit snd_sb16_isa_probe(
else {
static int possible_ports[] = {0x220, 0x240, 0x260, 0x280};
int i;
+ int uninitialized_var(err);
+
for (i = 0; i < ARRAY_SIZE(possible_ports); i++) {
port[dev] = possible_ports[i];
err = snd_sb16_isa_probe1(dev, pdev);
drivers/mmc/core/bus.c: In function $B!F(Jmmc_bus_uevent$B!G(J:
drivers/mmc/core/bus.c:77: warning: unused variable $B!F(Jlength$B!G(J
drivers/mmc/core/bus.c:77: warning: unused variable $B!F(Ji$B!G(J
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/mmc/core/bus.c | 1 -
1 file changed, 1 deletion(-)
--- linux-2.6.23-rc4-mm1/drivers/mmc/core/bus.c~fix 2007-09-02 19:18:05.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/mmc/core/bus.c 2007-09-02 19:18:08.000000000 +0530
@@ -74,7 +74,6 @@ mmc_bus_uevent(struct device *dev, struc
{
struct mmc_card *card = dev_to_mmc_card(dev);
const char *type;
- int i = 0, length = 0;
switch (card->type) {
case MMC_TYPE_MMC:
On 02/09/07, Satyam Sharma <[email protected]> wrote:
>
> I decided to ruin my Sunday with an utterly pointless activity --
> waging war on -mm build warnings. Some of the code I touched belonged
> to grotty, unused, dying drivers, but still, the end result was that
> I can now only see 5 warnings remaining on my typical .config (those
> have to do with pci_{find,enable}_device deprecation and __must_check
> on sysfs_create_{file,link} so I'll just leave 'em for now).
>
> Some were unused variable warnings, some uninitialized (few of those
> real bugs, but most not), some were "function defined but not used",
> some function argument signedness mismatches, some __must_check fixes,
> some tricky, but most just trivial.
>
> So till the next kernel release adds back shiny new build warnings,
> here's to some temporary peace ...
Great work Satyam.
Personally I hate all the build warnings we get and, just like you,
once in a while go on a warning killing spree. It's just so much
easier to spot the real problems when the build doesn't spew a ton of
pointless warnings.
Thank you for doing this, I hope some of your patches get merged.
Btw; it would be easier to see if one has got all the patches if you
numbered your patch series with the usual "[PATCH XX/YY]".
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
drivers/message/i2o/exec-osm.c:539: warning: $B!F(Ji2o_exec_lct_notify$B!G(J defined but not used
comes when CONFIG_I2O_LCT_NOTIFY_ON_CHANGES=n, because its only callsite
is #ifdef'ed as such. So let's #ifdef the function definition also. Also
move the definition to before the callsite, to get rid of forward prototype.
Signed-off-by: Satyam Sharma <[email protected]>
---
I have no clue who owns this subsystem, and recent git history is all
over the place.
drivers/message/i2o/exec-osm.c | 94 ++++++++++++++++++++---------------------
1 file changed, 47 insertions(+), 47 deletions(-)
--- linux-2.6.23-rc4-mm1/drivers/message/i2o/exec-osm.c~fix 2007-09-02 19:25:55.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/message/i2o/exec-osm.c 2007-09-02 19:25:07.000000000 +0530
@@ -41,8 +41,6 @@
struct i2o_driver i2o_exec_driver;
-static int i2o_exec_lct_notify(struct i2o_controller *c, u32 change_ind);
-
/* global wait list for POST WAIT */
static LIST_HEAD(i2o_exec_wait_list);
@@ -369,6 +367,53 @@ static int i2o_exec_remove(struct device
return 0;
};
+#ifdef CONFIG_I2O_LCT_NOTIFY_ON_CHANGES
+/**
+ * i2o_exec_lct_notify - Send a asynchronus LCT NOTIFY request
+ * @c: I2O controller to which the request should be send
+ * @change_ind: change indicator
+ *
+ * This function sends a LCT NOTIFY request to the I2O controller with
+ * the change indicator change_ind. If the change_ind == 0 the controller
+ * replies immediately after the request. If change_ind > 0 the reply is
+ * send after change indicator of the LCT is > change_ind.
+ */
+static int i2o_exec_lct_notify(struct i2o_controller *c, u32 change_ind)
+{
+ i2o_status_block *sb = c->status_block.virt;
+ struct device *dev;
+ struct i2o_message *msg;
+
+ mutex_lock(&c->lct_lock);
+
+ dev = &c->pdev->dev;
+
+ if (i2o_dma_realloc
+ (dev, &c->dlct, le32_to_cpu(sb->expected_lct_size), GFP_KERNEL))
+ return -ENOMEM;
+
+ msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
+ if (IS_ERR(msg))
+ return PTR_ERR(msg);
+
+ msg->u.head[0] = cpu_to_le32(EIGHT_WORD_MSG_SIZE | SGL_OFFSET_6);
+ msg->u.head[1] = cpu_to_le32(I2O_CMD_LCT_NOTIFY << 24 | HOST_TID << 12 |
+ ADAPTER_TID);
+ msg->u.s.icntxt = cpu_to_le32(i2o_exec_driver.context);
+ msg->u.s.tcntxt = cpu_to_le32(0x00000000);
+ msg->body[0] = cpu_to_le32(0xffffffff);
+ msg->body[1] = cpu_to_le32(change_ind);
+ msg->body[2] = cpu_to_le32(0xd0000000 | c->dlct.len);
+ msg->body[3] = cpu_to_le32(c->dlct.phys);
+
+ i2o_msg_post(c, msg);
+
+ mutex_unlock(&c->lct_lock);
+
+ return 0;
+};
+#endif
+
/**
* i2o_exec_lct_modified - Called on LCT NOTIFY reply
* @_work: work struct for a specific controller
@@ -525,51 +570,6 @@ int i2o_exec_lct_get(struct i2o_controll
return rc;
}
-/**
- * i2o_exec_lct_notify - Send a asynchronus LCT NOTIFY request
- * @c: I2O controller to which the request should be send
- * @change_ind: change indicator
- *
- * This function sends a LCT NOTIFY request to the I2O controller with
- * the change indicator change_ind. If the change_ind == 0 the controller
- * replies immediately after the request. If change_ind > 0 the reply is
- * send after change indicator of the LCT is > change_ind.
- */
-static int i2o_exec_lct_notify(struct i2o_controller *c, u32 change_ind)
-{
- i2o_status_block *sb = c->status_block.virt;
- struct device *dev;
- struct i2o_message *msg;
-
- mutex_lock(&c->lct_lock);
-
- dev = &c->pdev->dev;
-
- if (i2o_dma_realloc
- (dev, &c->dlct, le32_to_cpu(sb->expected_lct_size), GFP_KERNEL))
- return -ENOMEM;
-
- msg = i2o_msg_get_wait(c, I2O_TIMEOUT_MESSAGE_GET);
- if (IS_ERR(msg))
- return PTR_ERR(msg);
-
- msg->u.head[0] = cpu_to_le32(EIGHT_WORD_MSG_SIZE | SGL_OFFSET_6);
- msg->u.head[1] = cpu_to_le32(I2O_CMD_LCT_NOTIFY << 24 | HOST_TID << 12 |
- ADAPTER_TID);
- msg->u.s.icntxt = cpu_to_le32(i2o_exec_driver.context);
- msg->u.s.tcntxt = cpu_to_le32(0x00000000);
- msg->body[0] = cpu_to_le32(0xffffffff);
- msg->body[1] = cpu_to_le32(change_ind);
- msg->body[2] = cpu_to_le32(0xd0000000 | c->dlct.len);
- msg->body[3] = cpu_to_le32(c->dlct.phys);
-
- i2o_msg_post(c, msg);
-
- mutex_unlock(&c->lct_lock);
-
- return 0;
-};
-
/* Exec OSM driver struct */
struct i2o_driver i2o_exec_driver = {
.name = OSM_NAME,
drivers/message/fusion/mptctl.c: In function $B!F(Jmptctl_mpt_command$B!G(J:
drivers/message/fusion/mptctl.c:1764: warning: $B!F(JbufIn.len$B!G(J may be used uninitialized in this function
drivers/message/fusion/mptctl.c:1765: warning: $B!F(JbufOut.len$B!G(J may be used uninitialized in this function
come because gcc gets confused by some "goto" statements in above function.
The warnings have been verified to be bogus, however, the function does
initialize these later (after the offending goto's) in the function anyway.
So let's move those initializations to top of function, thereby also shutting
up these warnings.
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/message/fusion/mptctl.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--- linux-2.6.23-rc4-mm1/drivers/message/fusion/mptctl.c~fix 2007-09-02 21:51:14.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/message/fusion/mptctl.c 2007-09-02 21:54:25.000000000 +0530
@@ -1773,7 +1773,10 @@ mptctl_do_mpt_command (struct mpt_ioctl_
ulong timeout;
struct scsi_device *sdev;
+ /* bufIn and bufOut are used for user to kernel space transfers
+ */
bufIn.kptr = bufOut.kptr = NULL;
+ bufIn.len = bufOut.len = 0;
if (((iocnum = mpt_verify_adapter(karg.hdr.iocnum, &ioc)) < 0) ||
(ioc == NULL)) {
@@ -2107,11 +2110,6 @@ mptctl_do_mpt_command (struct mpt_ioctl_
psge = (char *) (((int *) mf) + karg.dataSgeOffset);
flagsLength = 0;
- /* bufIn and bufOut are used for user to kernel space transfers
- */
- bufIn.kptr = bufOut.kptr = NULL;
- bufIn.len = bufOut.len = 0;
-
if (karg.dataOutSize > 0)
sgSize ++;
On Sunday 02 September 2007 22:13:49 Satyam Sharma wrote:
>
> drivers/scsi/ips.c: In function ‘ips_register_scsi’:
> drivers/scsi/ips.c:6869:
> warning: ignoring return value of ‘scsi_add_host’, declared with attribute warn_unused_result
>
> scsi_add_host() is __must_check, so let's check it's return and cleanup
> appropriately on errors.
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
[snip]
Something seems to be wrong either at your end or mine
> + IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to add SCSI host¥n");
This should end with a newline "\n" but I'm seeing "¥n" ...
/Jesper
sound/isa/es18xx.c: In function $B!F(Jsnd_es18xx_isa_probe$B!G(J:
sound/isa/es18xx.c:2251: warning: $B!F(Jerr$B!G(J may be used uninitialized in this function
gcc is a sad, sad compiler. This warning is bogus so let's shut it up.
Signed-off-by: Satyam Sharma <[email protected]>
---
sound/isa/es18xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-2.6.23-rc4-mm1/sound/isa/es18xx.c~fix 2007-09-02 21:16:54.000000000 +0530
+++ linux-2.6.23-rc4-mm1/sound/isa/es18xx.c 2007-09-02 21:30:13.000000000 +0530
@@ -2248,7 +2248,6 @@ static int __devinit snd_es18xx_isa_prob
static int __devinit snd_es18xx_isa_probe(struct device *pdev, unsigned int dev)
{
- int err;
static int possible_irqs[] = {5, 9, 10, 7, 11, 12, -1};
static int possible_dmas[] = {1, 0, 3, 5, -1};
@@ -2276,6 +2275,8 @@ static int __devinit snd_es18xx_isa_prob
} else {
static unsigned long possible_ports[] = {0x220, 0x240, 0x260, 0x280};
int i;
+ int uninitialized_var(err);
+
for (i = 0; i < ARRAY_SIZE(possible_ports); i++) {
port[dev] = possible_ports[i];
err = snd_es18xx_isa_probe1(dev, pdev);
drivers/md/dm-exception-store.c: In function $B!F(Jpersistent_read_metadata$B!G(J:
drivers/md/dm-exception-store.c:452: warning: $B!F(Jnew_snapshot$B!G(J may be used uninitialized in this function
drivers/md/dm-ioctl.c: In function $B!F(Jctl_ioctl$B!G(J:
drivers/md/dm-ioctl.c:1407: warning: $B!F(Jparam$B!G(J may be used uninitialized in this function
[ For these, I'd like to especially add -- shame on you, gcc! ]
drivers/md/dm-table.c: In function $B!F(Jdm_get_device$B!G(J:
drivers/md/dm-table.c:472: warning: $B!F(Jdev$B!G(J may be used uninitialized in this function
are all verified to be bogus warnings. Let's shut them up.
---
drivers/md/dm-exception-store.c | 3 ++-
drivers/md/dm-ioctl.c | 2 +-
drivers/md/dm-table.c | 2 +-
3 file changed, 4 insertions(+), 3 deletion(-)
--- linux-2.6.23-rc4-mm1/drivers/md/dm-exception-store.c~fix 2007-09-03 01:05:12.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/md/dm-exception-store.c 2007-09-03 01:11:37.000000000 +0530
@@ -449,7 +449,8 @@ static void persistent_destroy(struct ex
static int persistent_read_metadata(struct exception_store *store)
{
- int r, new_snapshot;
+ int r;
+ int uninitialized_var(new_snapshot);
struct pstore *ps = get_info(store);
/*
--- linux-2.6.23-rc4-mm1/drivers/md/dm-ioctl.c~fix 2007-09-03 01:04:58.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/md/dm-ioctl.c 2007-09-03 01:07:04.000000000 +0530
@@ -1404,7 +1404,7 @@ static int ctl_ioctl(struct inode *inode
{
int r = 0;
unsigned int cmd;
- struct dm_ioctl *param;
+ struct dm_ioctl * uninitialized_var(param);
struct dm_ioctl __user *user = (struct dm_ioctl __user *) u;
ioctl_fn fn = NULL;
size_t param_size;
--- linux-2.6.23-rc4-mm1/drivers/md/dm-table.c~fix 2007-09-03 00:56:05.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/md/dm-table.c 2007-09-03 00:57:15.000000000 +0530
@@ -469,7 +469,7 @@ static int __table_get_device(struct dm_
int mode, struct dm_dev **result)
{
int r;
- dev_t dev;
+ dev_t uninitialized_var(dev);
struct dm_dev *dd;
unsigned int major, minor;
drivers/scsi/dc395x.c: In function $B!F(Jdc395x_init_one$B!G(J:
drivers/scsi/dc395x.c:4272: warning: $B!F(Jptr$B!G(J may be used uninitialized in this function
has been verified to be a bogus warning. Let's shut it up.
Signed-off-by: Satyam Sharma <[email protected]>
---
drivers/scsi/dc395x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c~fix 2007-09-02 20:57:51.000000000 +0530
+++ linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c 2007-09-02 21:10:49.000000000 +0530
@@ -4269,7 +4269,7 @@ static int __devinit adapter_sg_tables_a
const unsigned srbs_per_page = PAGE_SIZE/SEGMENTX_LEN;
int srb_idx = 0;
unsigned i = 0;
- struct SGentry *ptr;
+ struct SGentry * uninitialized_var(ptr);
for (i = 0; i < DC395x_MAX_SRB_CNT; i++)
acb->srb_array[i].segment_x = NULL;
These build warnings:
In file included from include/asm/thread_info.h:16,
from include/linux/thread_info.h:21,
from include/linux/preempt.h:9,
from include/linux/spinlock.h:49,
from include/linux/vmalloc.h:4,
from arch/i386/boot/compressed/misc.c:14:
include/asm/processor.h: In function $B!F(Jcpuid_count$B!G(J:
include/asm/processor.h:615: warning: pointer targets in passing argument 1 of $B!F(Jnative_cpuid$B!G(J differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 2 of $B!F(Jnative_cpuid$B!G(J differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 3 of $B!F(Jnative_cpuid$B!G(J differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 4 of $B!F(Jnative_cpuid$B!G(J differ in signedness
come because the arguments have been specified as pointers to (signed) int
types, not unsigned. So let's specify those as unsigned. Do some codingstyle
here and there while at it.
Signed-off-by: Satyam Sharma <[email protected]>
---
include/asm-i386/processor.h | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
--- linux-2.6.23-rc4-mm1/include/asm-i386/processor.h~fix 2007-09-02 23:54:23.000000000 +0530
+++ linux-2.6.23-rc4-mm1/include/asm-i386/processor.h 2007-09-02 23:57:46.000000000 +0530
@@ -599,7 +599,9 @@ static inline void load_esp0(struct tss_
* clear %ecx since some cpus (Cyrix MII) do not set or clear %ecx
* resulting in stale register contents being returned.
*/
-static inline void cpuid(unsigned int op, unsigned int *eax, unsigned int *ebx, unsigned int *ecx, unsigned int *edx)
+static inline void cpuid(unsigned int op,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
{
*eax = op;
*ecx = 0;
@@ -607,8 +609,9 @@ static inline void cpuid(unsigned int op
}
/* Some CPUID calls want 'count' to be placed in ecx */
-static inline void cpuid_count(int op, int count, int *eax, int *ebx, int *ecx,
- int *edx)
+static inline void cpuid_count(unsigned int op, int count,
+ unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
{
*eax = op;
*ecx = count;
On 02/09/07, Satyam Sharma <[email protected]> wrote:
>
> drivers/usb/misc/sisusbvga/sisusb.c: In function 'usb_sisusb_init':
> drivers/usb/misc/sisusbvga/sisusb.c:3321: warning: unused variable 'sisusb'
> drivers/usb/misc/sisusbvga/sisusb.c:3320: warning: unused variable 'retval'
>
> are trivially solved by getting rid of the unused variables.
>
> drivers/usb/misc/sisusbvga/sisusb.c: In function 'sisusb_open':
> drivers/usb/misc/sisusbvga/sisusb.c:2444: warning: 'sisusb' is used uninitialized in this function
>
> is a genuine bug (which will cause oops). We cannot use "sisusb" in
> error path for (!interface), because sisusb will itself be derived
> from "interface" later.
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
> ---
>
> drivers/usb/misc/sisusbvga/sisusb.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> --- linux-2.6.23-rc4-mm1/drivers/usb/misc/sisusbvga/sisusb.c(J~(Bfix 2007-09-02 19:06:01.000000000 +0530
> +++ linux-2.6.23-rc4-mm1/drivers/usb/misc/sisusbvga/sisusb.c 2007-09-02 19:10:52.000000000 +0530
> @@ -2440,10 +2440,8 @@ sisusb_open(struct inode *inode, struct
> struct usb_interface *interface;
> int subminor = iminor(inode);
>
> - if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
> - dev_err(&sisusb->sisusb_dev->dev, "Failed to find interface(J\(Bn");
Odd how in your patch the line ends with "(J\(Bn" but if I look in my
local copy of the source tree I see "\n".
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Hi Jesper,
On Sun, 2 Sep 2007, Jesper Juhl wrote:
>
> > - if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
> > - dev_err(&sisusb->sisusb_dev->dev, "Failed to find interface?n");
>
> Odd how in your patch the line ends with "?n" but if I look in my
> local copy of the source tree I see "\n".
Odd, indeed. I see correct '\n' in the mail I sent, but in your mail
here it's coming out wrong -- lkml.org shows badness as well. Hmmm,
"insert file" using ^R in alpine never gave any problems before ...
On Mon, 2007-09-03 at 01:41 +0530, Satyam Sharma wrote:
> net/wireless/sysfs.c:108: warning: ‘wiphy_uevent’ defined but not used
>
> when CONFIG_HOTPLUG=n is because the only usage site of this function
> is #ifdef'ed as such, so let's #ifdef the definition also.
>
> Signed-off-by: Satyam Sharma <[email protected]>
Looks good to me. The new style seems to be to do
#ifdef SOMETHING
code
#else
#define wiphy_uevent NULL
#endif
and then using it unconditionally, but I'm fine with both.
Acked-by: Johannes Berg <[email protected]>
> ---
>
> net/wireless/sysfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-2.6.23-rc4-mm1/net/wireless/sysfs.c‾fix 2007-09-02 20:06:11.000000000 +0530
> +++ linux-2.6.23-rc4-mm1/net/wireless/sysfs.c 2007-09-02 20:07:01.000000000 +0530
> @@ -104,11 +104,13 @@ static void wiphy_dev_release(struct dev
> cfg80211_dev_free(rdev);
> }
>
> +#ifdef CONFIG_HOTPLUG
> static int wiphy_uevent(struct device *dev, struct kobj_uevent_env *env)
> {
> /* TODO, we probably need stuff here */
> return 0;
> }
> +#endif
>
> struct class ieee80211_class = {
> .name = "ieee80211",
On 02/09/07, Satyam Sharma <[email protected]> wrote:
> Hi Jesper,
>
>
> On Sun, 2 Sep 2007, Jesper Juhl wrote:
> >
> > > - if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
> > > - dev_err(&sisusb->sisusb_dev->dev, "Failed to find interface?n");
> >
> > Odd how in your patch the line ends with "?n" but if I look in my
> > local copy of the source tree I see "\n".
>
> Odd, indeed. I see correct '\n' in the mail I sent, but in your mail
> here it's coming out wrong -- lkml.org shows badness as well. Hmmm,
> "insert file" using ^R in alpine never gave any problems before ...
Hmm, I often use pine and ^R myself without trouble - haven't upgraded
to alpine yet, but something certainly is wrong.
I see the badness in your mail both in the gmail web interface and
kmail. So I guess it is caused on your end.
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Satyam Sharma wrote:
> drivers/scsi/dc395x.c: In function ?dc395x_init_one?:
> drivers/scsi/dc395x.c:4272: warning: ?ptr? may be used uninitialized in this function
>
> has been verified to be a bogus warning. Let's shut it up.
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
> ---
>
> drivers/scsi/dc395x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c~fix 2007-09-02 20:57:51.000000000 +0530
> +++ linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c 2007-09-02 21:10:49.000000000 +0530
> @@ -4269,7 +4269,7 @@ static int __devinit adapter_sg_tables_a
> const unsigned srbs_per_page = PAGE_SIZE/SEGMENTX_LEN;
> int srb_idx = 0;
> unsigned i = 0;
> - struct SGentry *ptr;
> + struct SGentry * uninitialized_var(ptr);
For things like this, we need to see (perhaps a separate email in the
same thread) your build details: config, arch, compiler version, etc.
This warning does not appear on x86 or x86-64 here, with the current
version of gcc. And I'm not so sure we want to be adding these markers
for older versions of the compiler, or for what is ultimately a
temporary [situation | configuration] in the grand scheme of things.
I paid careful attention to my recent set of uninitialized_var() markers
(now upstream), making sure that they persisted across several compiler
versions, ensuring the warning was not a temporary optimizer bug quickly
remedied.
I hope to encourage similar diligence in others :) These markers have
the very-real potential downside of hiding bugs, so they should be used
sparingly.
Jeff
On Sun, 2 Sep 2007, Jesper Juhl wrote:
>
> Thank you for doing this, I hope some of your patches get merged.
>
> Btw; it would be easier to see if one has got all the patches if you
> numbered your patch series with the usual "[PATCH XX/YY]".
Hey, thanks ;-) There are 13 in all, I just felt lazy and simply used
^R from alpine in-reply-to the first mail one-after-another instead
of scripting it ...
On 02/09/07, Satyam Sharma <[email protected]> wrote:
>
>
> On Sun, 2 Sep 2007, Jesper Juhl wrote:
> >
> > Thank you for doing this, I hope some of your patches get merged.
> >
> > Btw; it would be easier to see if one has got all the patches if you
> > numbered your patch series with the usual "[PATCH XX/YY]".
>
> Hey, thanks ;-) There are 13 in all, I just felt lazy and simply used
> ^R from alpine in-reply-to the first mail one-after-another instead
> of scripting it ...
>
Hehe, I often do it that way myself, but just edit the subject of each
mail and manually put in the [PATCH XX/YY] bit.
It is just that sometimes mails get lost, so it's very nice to know
how many patches to expect in a series :)
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
Satyam Sharma wrote:
> drivers/scsi/ips.c: In function ?ips_register_scsi?:
> drivers/scsi/ips.c:6869:
> warning: ignoring return value of ?scsi_add_host?, declared with attribute warn_unused_result
>
> scsi_add_host() is __must_check, so let's check it's return and cleanup
> appropriately on errors.
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
> ---
>
> drivers/scsi/ips.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> --- linux-2.6.23-rc4-mm1/drivers/scsi/ips.c~fix 2007-09-02 20:21:27.000000000 +0530
> +++ linux-2.6.23-rc4-mm1/drivers/scsi/ips.c 2007-09-02 20:25:58.000000000 +0530
> @@ -6866,7 +6866,12 @@ ips_register_scsi(int index)
> sh->max_channel = ha->nbus - 1;
> sh->can_queue = ha->max_cmds - 1;
>
> - scsi_add_host(sh, NULL);
> + if (scsi_add_host(sh, NULL)) {
> + IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to add SCSI host\n");
> + free_irq(ha->irq, ha);
> + scsi_host_put(sh);
> + return -1;
ACK, as long as you add a comment to this code and the request_irq()
failure code: the oldha/old irq is not restored upon failure, in this
function.
That is a pre-existing bug, and not your fault, but your patch
"continues in the same buggy tradition" :) We should at least note the
FIXME at each error handling code branch.
Ideally you or somebody should do a detailed analysis to
a) (preferably) get rid of the silly oldha/ double-irq-request weirdness
and make it look like other drivers,
or,
b) analyze the code and see what it takes to _really_ unwind the error.
Also, I would recommend moving the error handling code to the end of the
function and using the standard 'goto' approach for function error
handling. This eliminates the duplicate scsi_host_put() calls upon error.
Jeff
Satyam Sharma wrote:
> drivers/md/dm-exception-store.c: In function ?persistent_read_metadata?:
> drivers/md/dm-exception-store.c:452: warning: ?new_snapshot? may be used uninitialized in this function
>
> drivers/md/dm-ioctl.c: In function ?ctl_ioctl?:
> drivers/md/dm-ioctl.c:1407: warning: ?param? may be used uninitialized in this function
>
> [ For these, I'd like to especially add -- shame on you, gcc! ]
>
> drivers/md/dm-table.c: In function ?dm_get_device?:
> drivers/md/dm-table.c:472: warning: ?dev? may be used uninitialized in this function
>
> are all verified to be bogus warnings. Let's shut them up.
>
> ---
>
> drivers/md/dm-exception-store.c | 3 ++-
> drivers/md/dm-ioctl.c | 2 +-
> drivers/md/dm-table.c | 2 +-
> 3 file changed, 4 insertions(+), 3 deletion(-)
same comment as with the last uninit'd var patch: these markers should
be used sparingly. Try it on multiple compiler versions, see if it's a
new behavior.
Quite realistically, you might actually be finding gcc bugs, implying
the proper path is to file a gcc bug report (and they are _very_
diligent about handling these, its impressive) rather than to patch the
Linux kernel.
Overall, for any uninitialized_var() patch, we need more info on
platform/compiler version/analysis methods/etc.
Jeff
On Sun, 2 Sep 2007, Jeff Garzik wrote:
>
> Satyam Sharma wrote:
> > drivers/scsi/dc395x.c: In function $B!F(Jdc395x_init_one$B!G(J:
> > drivers/scsi/dc395x.c:4272: warning: $B!F(Jptr$B!G(J may be used uninitialized in this
> > function
> >
> > has been verified to be a bogus warning. Let's shut it up.
> >
> > Signed-off-by: Satyam Sharma <[email protected]>
> >
> > ---
> >
> > drivers/scsi/dc395x.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > --- linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c~fix 2007-09-02
> > 20:57:51.000000000 +0530
> > +++ linux-2.6.23-rc4-mm1/drivers/scsi/dc395x.c 2007-09-02
> > 21:10:49.000000000 +0530
> > @@ -4269,7 +4269,7 @@ static int __devinit adapter_sg_tables_a
> > const unsigned srbs_per_page = PAGE_SIZE/SEGMENTX_LEN;
> > int srb_idx = 0;
> > unsigned i = 0;
> > - struct SGentry *ptr;
> > + struct SGentry * uninitialized_var(ptr);
>
> For things like this, we need to see (perhaps a separate email in the same
> thread) your build details: config, arch, compiler version, etc.
I'll post the info as a reply to the first mail in this series. I have
fairly recent gcc (4.1.1) and I don't see us dropping support for it in
the next few years.
> I paid careful attention to my recent set of uninitialized_var() markers (now
> upstream), making sure that they persisted across several compiler versions,
> ensuring the warning was not a temporary optimizer bug quickly remedied.
>
> I hope to encourage similar diligence in others :) These markers have the
> very-real potential downside of hiding bugs, so they should be used sparingly.
Well, uninitialized_var() is definitely more greppable than "= 0" which
code all over does all the time :-) Anyway, the point is that the
warnings are bogus anyway (hence maintainers Cc:'ed so that they can
confirm if I arrived at the conclusion correctly) /and/ that
uninitialized_var() stands out sorely, so I don't see bugs getting
"hidden" at all -- in fact I did find two bugs during this process, and
fixed them accordingly.
Thanks,
Satyam
Satyam Sharma wrote:
> I'll post the info as a reply to the first mail in this series. I have
> fairly recent gcc (4.1.1) and I don't see us dropping support for it in
> the next few years.
What's important is not support lifetime, but whether or not the warning
persists through version 4.1.2, 4.1.3, etc.
We don't want to add markers for compiler quirks that come and go.
The current markers tend to exist for a class of problems that gcc
fundamentally has a tough time "seeing." Different optimizer behaviors
from compiler version to compiler version are just noise[1].
Jeff
[1] sometimes quite literally, in the case of compiler warnings.
On Sun, 2 Sep 2007, Jeff Garzik wrote:
> Satyam Sharma wrote:
> > drivers/md/dm-exception-store.c: In function $B!F(Jpersistent_read_metadata$B!G(J:
> > drivers/md/dm-exception-store.c:452: warning: $B!F(Jnew_snapshot$B!G(J may be used
> > uninitialized in this function
> >
> > drivers/md/dm-ioctl.c: In function $B!F(Jctl_ioctl$B!G(J:
> > drivers/md/dm-ioctl.c:1407: warning: $B!F(Jparam$B!G(J may be used uninitialized in
> > this function
> >
> > [ For these, I'd like to especially add -- shame on you, gcc! ]
> >
> > drivers/md/dm-table.c: In function $B!F(Jdm_get_device$B!G(J:
> > drivers/md/dm-table.c:472: warning: $B!F(Jdev$B!G(J may be used uninitialized in this
> > function
> >
> > are all verified to be bogus warnings. Let's shut them up.
> >
> > ---
> >
> > drivers/md/dm-exception-store.c | 3 ++-
> > drivers/md/dm-ioctl.c | 2 +-
> > drivers/md/dm-table.c | 2 +-
> > 3 file changed, 4 insertions(+), 3 deletion(-)
>
> same comment as with the last uninit'd var patch: these markers should be
> used sparingly. Try it on multiple compiler versions, see if it's a new
> behavior.
>
> Quite realistically, you might actually be finding gcc bugs,
Definitely -- a lot of these patches are purely a result of gcc's shoddy
inadequacies. I do intend filing reports with gcc, however ...
> implying the
> proper path is to file a gcc bug report (and they are _very_ diligent about
> handling these, its impressive) rather than to patch the Linux kernel.
My experience (regarding gcc's diligence in dealing with bug reports) has
been otherwise :-)
Anyway, I once had a _very_ similar discussion with Andrew (regarding some
other gcc bug) previously, and the (completely correct) point he made was
that: firstly, there's no guarantee they'll get this sorted out asap, and
even if they did, we'll still not be dropping support for older gcc
versions anyway ...
On Sunday 02 September 2007 21:23:16 Jesper Juhl wrote:
> On 02/09/07, Satyam Sharma <[email protected]> wrote:
> > Hi Jesper,
> >
> > On Sun, 2 Sep 2007, Jesper Juhl wrote:
> > > > - if (!(interface = usb_find_interface(&sisusb_driver,
> > > > subminor))) { - dev_err(&sisusb->sisusb_dev->dev,
> > > > "Failed to find interface?n");
> > >
> > > Odd how in your patch the line ends with "?n" but if I look in my
> > > local copy of the source tree I see "\n".
> >
> > Odd, indeed. I see correct '\n' in the mail I sent, but in your mail
> > here it's coming out wrong -- lkml.org shows badness as well. Hmmm,
> > "insert file" using ^R in alpine never gave any problems before ...
The encoding is set to ISO-2022-JP, this is probably breaking things.
--
Cheers,
Alistair.
137/1 Warrender Park Road, Edinburgh, UK.
On Sun, 2 Sep 2007, Alistair John Strachan wrote:
> On Sunday 02 September 2007 21:23:16 Jesper Juhl wrote:
> > On 02/09/07, Satyam Sharma <[email protected]> wrote:
> > > Hi Jesper,
> > >
> > > On Sun, 2 Sep 2007, Jesper Juhl wrote:
> > > > > - if (!(interface = usb_find_interface(&sisusb_driver,
> > > > > subminor))) { - dev_err(&sisusb->sisusb_dev->dev,
> > > > > "Failed to find interface?n");
> > > >
> > > > Odd how in your patch the line ends with "?n" but if I look in my
> > > > local copy of the source tree I see "\n".
> > >
> > > Odd, indeed. I see correct '\n' in the mail I sent, but in your mail
> > > here it's coming out wrong -- lkml.org shows badness as well. Hmmm,
> > > "insert file" using ^R in alpine never gave any problems before ...
>
> The encoding is set to ISO-2022-JP, this is probably breaking things.
??? I have no clue how/why/when that suddenly happened.
I clearly see "Content-Type: TEXT/PLAIN; charset=us-ascii" for the first
mail I sent out -- but all the replies seem to have somehow got this
weirdness. Will close and restart my alpine session, just in case ...
thanks for heads-up, Alistair.
On 09/02/2007 10:15 PM, Satyam Sharma wrote:
> sound/isa/sb16/sb16.c: In function ‘snd_sb16_isa_probe’:
Blah. Your message has:
Content-Type: TEXT/PLAIN; charset=iso-2022-jp
This apparently is caused by a combination of GCC using groovy UTF tickmarks
in its error messages when in a UTF locale and alpine believing it to be a
great idea to automatically try for the "simplest" character set it can
encode the content in. No idea why that means that iso-2022-jp is picked,
but it is.
While I could actually read the message this time you should see what
iso-2022-jp does to my font. It's scary. Best solution as far as I'm
concerned is slap a few GCC developers (not that it wil help, but it'll
certainly feel good) and then teach alpine to go for UTF-8 directly if
US-ASCII won't do.
As to the content of this patch -- I'd almost say it's better to live with
the warning than with that unitialized_var() thing. That ARRAY_SIZE is very
much a compile time constant, so exactly how dumb must GCC get before we get
to say to here and no further?
> --- linux-2.6.23-rc4-mm1/sound/isa/sb/sb16.c~fix 2007-09-02 21:41:51.000000000 +0530
> +++ linux-2.6.23-rc4-mm1/sound/isa/sb/sb16.c 2007-09-02 21:42:56.000000000 +0530
> @@ -556,7 +556,6 @@ static int __devinit snd_sb16_isa_match(
>
> static int __devinit snd_sb16_isa_probe(struct device *pdev, unsigned int dev)
> {
> - int err;
> static int possible_irqs[] = {5, 9, 10, 7, -1};
> static int possible_dmas8[] = {1, 3, 0, -1};
> static int possible_dmas16[] = {5, 6, 7, -1};
> @@ -585,6 +584,8 @@ static int __devinit snd_sb16_isa_probe(
> else {
> static int possible_ports[] = {0x220, 0x240, 0x260, 0x280};
> int i;
> + int uninitialized_var(err);
> +
> for (i = 0; i < ARRAY_SIZE(possible_ports); i++) {
> port[dev] = possible_ports[i];
> err = snd_sb16_isa_probe1(dev, pdev);
Rene.
On Mon, 3 Sep 2007, Satyam Sharma wrote:
>
> On Sun, 2 Sep 2007, Alistair John Strachan wrote:
> >
> > The encoding is set to ISO-2022-JP, this is probably breaking things.
>
> ??? I have no clue how/why/when that suddenly happened.
>
> I clearly see "Content-Type: TEXT/PLAIN; charset=us-ascii" for the first
> mail I sent out -- but all the replies seem to have somehow got this
> weirdness.
OK, I've got it figured out now. What happened is this -- alpine has this
nifty feature that (if not explicitly set) it will automatically adjust
the content-type encoding of outgoing mails depending upon the kind of
text it sees written in the mail. Now when I normally post (eg. the first
mail in this set), it naturally goes out as US-ASCII. What was special
about the other mails was that each contained GCC's error/warning output.
And if you've noticed it produces that totally pointless special non-
US-ASCII 'quotes' around wherever it points out the name of an identifier
-- and that confused alpine into thinking I was writing in Japanese (!)
Anyway, I've explicitly set all mails to be sent out as US-ASCII only
now, so hopefully things will be ok ...
On 09/02/2007 10:21 PM, Satyam Sharma wrote:
> sound/isa/es18xx.c: In function ‘snd_es18xx_isa_probe’:
> sound/isa/es18xx.c:2251: warning: ‘err’ may be used uninitialized in this function
>
> gcc is a sad, sad compiler. This warning is bogus so let's shut it up.
Same situation, same comment (and same charset :-( ) as the sb16 one.
Rene.
On Mon, 3 Sep 2007, Rene Herman wrote:
>
> On 09/02/2007 10:15 PM, Satyam Sharma wrote:
>
> > sound/isa/sb16/sb16.c: In function ‘snd_sb16_isa_probe’:
>
> Blah. Your message has:
>
> Content-Type: TEXT/PLAIN; charset=iso-2022-jp
>
> This apparently is caused by a combination of GCC using groovy UTF tickmarks
> in its error messages when in a UTF locale and alpine believing it to be a
> great idea to automatically try for the "simplest" character set it can encode
> the content in. No idea why that means that iso-2022-jp is picked, but it is.
Yeah, precisely.
> As to the content of this patch -- I'd almost say it's better to live with the
> warning than with that unitialized_var() thing. That ARRAY_SIZE is very much a
> compile time constant, so exactly how dumb must GCC get before we get to say
> to here and no further?
Pretty dumb indeed -- in fact that's the case with 4 patches in this
series. Like Jeff said, that (gcc's) behaviour has likely even improved
w.r.t. later versions, so I guess it's fine if these 4 patches are not
applied -- I'll leave it upto the maintainers.
On Sun, 2 Sep 2007, Jeff Garzik wrote:
> Satyam Sharma wrote:
> > drivers/scsi/ips.c: In function ‘ips_register_scsi’:
> > drivers/scsi/ips.c:6869:
> > warning: ignoring return value of ‘scsi_add_host’, declared with attribute
> > warn_unused_result
> >
> > scsi_add_host() is __must_check, so let's check it's return and cleanup
> > appropriately on errors.
> >
> > Signed-off-by: Satyam Sharma <[email protected]>
> >
> > ---
> >
> > drivers/scsi/ips.c | 7 ++++++-
> > 1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > --- linux-2.6.23-rc4-mm1/drivers/scsi/ips.c~fix 2007-09-02
> > 20:21:27.000000000 +0530
> > +++ linux-2.6.23-rc4-mm1/drivers/scsi/ips.c 2007-09-02 20:25:58.000000000
> > +0530
> > @@ -6866,7 +6866,12 @@ ips_register_scsi(int index)
> > sh->max_channel = ha->nbus - 1;
> > sh->can_queue = ha->max_cmds - 1;
> > - scsi_add_host(sh, NULL);
> > + if (scsi_add_host(sh, NULL)) {
> > + IPS_PRINTK(KERN_WARNING, ha->pcidev, "Unable to add SCSI
> > host\n");
> > + free_irq(ha->irq, ha);
> > + scsi_host_put(sh);
> > + return -1;
>
> ACK, as long as you add a comment to this code and the request_irq() failure
> code: the oldha/old irq is not restored upon failure, in this function.
>
> That is a pre-existing bug, and not your fault, but your patch "continues in
> the same buggy tradition" :) We should at least note the FIXME at each error
> handling code branch.
True, I did notice that (as well as the coding style issue you pointed
out) when making the patch, but felt lazy to actually set it right fully.
And that (when it's done) should be a separate patch anyway.
On Mon, 3 Sep 2007 01:46:20 +0530 (IST)
Satyam Sharma <[email protected]> wrote:
>
> drivers/mmc/core/bus.c: In function ‘mmc_bus_uevent’:
> drivers/mmc/core/bus.c:77: warning: unused variable ‘length’
> drivers/mmc/core/bus.c:77: warning: unused variable ‘i’
>
> Signed-off-by: Satyam Sharma <[email protected]>
>
That particular piece of code seems to be a war zone in -mm right now.
I'll leave it be until we start merging this upstream.
Rgds
--
-- Pierre Ossman
Linux kernel, MMC maintainer http://www.kernel.org
PulseAudio, core developer http://pulseaudio.org
rdesktop, core developer http://www.rdesktop.org
This is a note to let you know that I've just added the patch titled
Subject: USB: sisusbvga: Fix bug and build warnings
to my gregkh-2.6 tree. Its filename is
usb-sisusbvga-fix-bug-and-build-warnings.patch
This tree can be found at
http://www.kernel.org/pub/linux/kernel/people/gregkh/gregkh-2.6/patches/
>From [email protected] Sun Sep 2 12:54:28 2007
From: Satyam Sharma <[email protected]>
Date: Mon, 3 Sep 2007 01:37:31 +0530 (IST)
Subject: USB: sisusbvga: Fix bug and build warnings
To: Linux Kernel Mailing List <[email protected]>
Cc: Thomas Winischhofer <[email protected]>, Greg Kroah-Hartman <[email protected]>, [email protected]
Message-ID: <[email protected]>
drivers/usb/misc/sisusbvga/sisusb.c: In function usb_sisusb_init:
drivers/usb/misc/sisusbvga/sisusb.c:3321: warning: unused variable sisusb
drivers/usb/misc/sisusbvga/sisusb.c:3320: warning: unused variable retval
are trivially solved by getting rid of the unused variables.
drivers/usb/misc/sisusbvga/sisusb.c: In function sisusb_open
drivers/usb/misc/sisusbvga/sisusb.c:2444: warning: sisusb is used uninitialized in this function
is a genuine bug (which will cause oops). We cannot use "sisusb" in
error path for (!interface), because sisusb will itself be derived
from "interface" later.
Signed-off-by: Satyam Sharma <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
drivers/usb/misc/sisusbvga/sisusb.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
--- a/drivers/usb/misc/sisusbvga/sisusb.c
+++ b/drivers/usb/misc/sisusbvga/sisusb.c
@@ -2440,10 +2440,8 @@ sisusb_open(struct inode *inode, struct
struct usb_interface *interface;
int subminor = iminor(inode);
- if (!(interface = usb_find_interface(&sisusb_driver, subminor))) {
- dev_err(&sisusb->sisusb_dev->dev, "Failed to find interface\n");
+ if (!(interface = usb_find_interface(&sisusb_driver, subminor)))
return -ENODEV;
- }
if (!(sisusb = usb_get_intfdata(interface)))
return -ENODEV;
Patches currently in gregkh-2.6 which might be from [email protected] are
driver/sysfs-remove-first-pass-at-shadow-directory-support.patch
driver/sysfs-introduce-sysfs_rename_mutex.patch
driver/sysfs-cosmetic-changes-in-sysfs_lookup.patch
driver/sysfs-make-sysfs_add-remove_one-call-link-unlink_sibling-implictly.patch
driver/sysfs-make-sysfs_add_one-automatically-check-for-duplicate-entry.patch
driver/sysfs-make-sysfs_addrm_finish-return-void.patch
driver/sysfs-simplify-sysfs_rename_dir.patch
driver/sysfs-kill-sysfs_flag_removed.patch
driver/sysfs-remove-s_dentry.patch
driver/sysfs-fix-i_mutex-locking-in-sysfs_get_dentry.patch
driver/sysfs-in-sysfs_lookup-don-t-open-code-sysfs_find_dirent.patch
driver/sysfs-make-sysfs_mount-static.patch
driver/sysfs-move-all-of-inode-initialization-into-sysfs_init_inode.patch
driver/sysfs-remove-sysfs_instantiate.patch
driver/sysfs-rewrite-rename-in-terms-of-sysfs-dirents.patch
driver/sysfs-rewrite-sysfs_drop_dentry.patch
driver/sysfs-rewrite-sysfs_move_dir-in-terms-of-sysfs-dirents.patch
driver/sysfs-simplify-readdir.patch
driver/sysfs-simply-sysfs_get_dentry.patch
driver/sysfs-use-kill_anon_super.patch
usb/usb-drivers-usb-serial-bus.c-fix-incompatible-pointer-type-warning.patch
usb/usb-sisusbvga-fix-bug-and-build-warnings.patch
On Sunday 02 September 2007 23:06, Rene Herman wrote:
> On 09/02/2007 10:15 PM, Satyam Sharma wrote:
>
> > sound/isa/sb16/sb16.c: In function ‘snd_sb16_isa_probe’:
>
> Blah. Your message has:
>
> Content-Type: TEXT/PLAIN; charset=iso-2022-jp
>
> This apparently is caused by a combination of GCC using groovy UTF tickmarks
> in its error messages when in a UTF locale and alpine believing it to be a
> great idea to automatically try for the "simplest" character set it can
> encode the content in. No idea why that means that iso-2022-jp is picked,
> but it is.
>
> While I could actually read the message this time you should see what
> iso-2022-jp does to my font. It's scary. Best solution as far as I'm
> concerned is slap a few GCC developers (not that it wil help, but it'll
> certainly feel good) and then teach alpine to go for UTF-8 directly if
> US-ASCII won't do.
rotfl.
Kindly give me permission to convert your email into gcc bugreport
and/or to forward it to gcc mailing list.
--
vda
On 09/20/2007 07:52 PM, Denys Vlasenko wrote:
> On Sunday 02 September 2007 23:06, Rene Herman wrote:
>> Blah. Your message has:
>>
>> Content-Type: TEXT/PLAIN; charset=iso-2022-jp
>>
>> This apparently is caused by a combination of GCC using groovy UTF tickmarks
>> in its error messages when in a UTF locale and alpine believing it to be a
>> great idea to automatically try for the "simplest" character set it can
>> encode the content in. No idea why that means that iso-2022-jp is picked,
>> but it is.
>>
>> While I could actually read the message this time you should see what
>> iso-2022-jp does to my font. It's scary. Best solution as far as I'm
>> concerned is slap a few GCC developers (not that it wil help, but it'll
>> certainly feel good) and then teach alpine to go for UTF-8 directly if
>> US-ASCII won't do.
>
> rotfl.
>
> Kindly give me permission to convert your email into gcc bugreport
> and/or to forward it to gcc mailing list.
Blessings be upon thou, oh courageous one...
Rene.