[-mm patchset] War on warnings

[-mm patchset] War on warnings

[Satyam Sharma]

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 ...

Re: [-mm patchset] War on warnings

[Jesper Juhl]

On 02/09/07, Satyam Sharma <satyam@infradead.org> 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 <jesper.juhl@gmail.com>
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

Re: [-mm patchset] War on warnings

[Satyam Sharma]

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 ...

Re: [-mm patchset] War on warnings

[Jesper Juhl]

On 02/09/07, Satyam Sharma <satyam@infradead.org> 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 <jesper.juhl@gmail.com>
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

[PATCH -mm] sisusbvga: Fix bug and build warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1678 bytes --]


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 <satyam@infradead.org>

---

 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

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Jesper Juhl]

On 02/09/07, Satyam Sharma <satyam@infradead.org> 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 <satyam@infradead.org>
>
> ---
>
>  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");

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".


-- 
Jesper Juhl <jesper.juhl@gmail.com>
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

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 542 bytes --]

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 ...

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Jesper Juhl]

On 02/09/07, Satyam Sharma <satyam@infradead.org> 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 <jesper.juhl@gmail.com>
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

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Alistair John Strachan]

On Sunday 02 September 2007 21:23:16 Jesper Juhl wrote:
> On 02/09/07, Satyam Sharma <satyam@infradead.org> 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.

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1165 bytes --]



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 <satyam@infradead.org> 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.

Re: [PATCH -mm] sisusbvga: Fix bug and build warnings

[Satyam Sharma]

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 ...

patch usb-sisusbvga-fix-bug-and-build-warnings.patch added to gregkh-2.6 tree

[gregkh]

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 satyam@infradead.org Sun Sep  2 12:54:28 2007
From: Satyam Sharma <satyam@infradead.org>
Date: Mon, 3 Sep 2007 01:37:31 +0530 (IST)
Subject: USB: sisusbvga: Fix bug and build warnings
To: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Cc: Thomas Winischhofer <thomas@winischhofer.net>,  Greg Kroah-Hartman <greg@kroah.com>, linux-usb-devel@lists.sourceforge.net
Message-ID: <alpine.LFD.0.999.0709030136140.29617@enigma.security.iitk.ac.in>


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 <satyam@infradead.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>


---
 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 satyam@infradead.org 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

[PATCH -mm] sunrpc svc: Shut up bogus uninitialized variable warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1467 bytes --]


net/sunrpc/svc.c: In function ‘__svc_create_thread’:
net/sunrpc/svc.c:550: warning: ‘oldmask.bits[0u]’ 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 <satyam@infradead.org>

---

 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;

[PATCH -mm] net/wireless/sysfs.c: Shut up build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 827 bytes --]


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 <satyam@infradead.org>

---

 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",

Re: [PATCH -mm] net/wireless/sysfs.c: Shut up build warning

[Johannes Berg]

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

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 <satyam@infradead.org>

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 <johannes@sipsolutions.net>

> ---
> 
>  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",

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

[PATCH -mm] drivers/usb/serial/bus.c: Fix incompatible pointer type warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1051 bytes --]


drivers/usb/serial/bus.c: In function ‘usb_serial_bus_deregister’:
drivers/usb/serial/bus.c:185:
warning: passing argument 1 of ‘free_dynids’ 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 <satyam@infradead.org>

---

 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

[PATCH -mm] IPS SCSI driver: Check return of scsi_add_host()

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 961 bytes --]


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 <satyam@infradead.org>

---

 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;

Re: [PATCH -mm] IPS SCSI driver: Check return of scsi_add_host()

[Jesper Juhl]

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 <satyam@infradead.org>
> 
[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

Re: [PATCH -mm] IPS SCSI driver: Check return of scsi_add_host()

[Jeff Garzik]

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 <satyam@infradead.org>
> 
> ---
> 
>  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

Re: [PATCH -mm] IPS SCSI driver: Check return of scsi_add_host()

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1630 bytes --]



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 <satyam@infradead.org>
> > 
> > ---
> > 
> >  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.

[PATCH -mm] sb16: Shut up uninitialized var build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1127 bytes --]


sound/isa/sb16/sb16.c: In function ‘snd_sb16_isa_probe’:
sound/isa/sb16/sb16.c:559: warning: ‘err’ may be used uninitialized in this function

is a bogus warning, so let's shut it up.

Signed-off-by: Satyam Sharma <satyam@infradead.org>

---

 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);

Re: [PATCH -mm] sb16: Shut up uninitialized var build warning

[Rene Herman]

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.

Re: [PATCH -mm] sb16: Shut up uninitialized var build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1105 bytes --]



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.

Re: [PATCH -mm] sb16: Shut up uninitialized var build warning

[Denys Vlasenko]

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

Re: [PATCH -mm] sb16: Shut up uninitialized var build warning

[Rene Herman]

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.

[PATCH -mm] drivers/mmc/core/bus.c: Fix unused var warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 711 bytes --]


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 <satyam@infradead.org>

---

 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:

Re: [PATCH -mm] drivers/mmc/core/bus.c: Fix unused var warning

[Pierre Ossman]

On Mon, 3 Sep 2007 01:46:20 +0530 (IST)
Satyam Sharma <satyam@infradead.org> 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 <satyam@infradead.org>
> 

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

[PATCH -mm] I2O: Fix "defined but not used" build warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4269 bytes --]


drivers/message/i2o/exec-osm.c:539: warning: ‘i2o_exec_lct_notify’ 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 <satyam@infradead.org>

---

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,

[PATCH -mm] mpt fusion: Shut up uninitialized variable warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1650 bytes --]


drivers/message/fusion/mptctl.c: In function ‘mptctl_mpt_command’:
drivers/message/fusion/mptctl.c:1764: warning: ‘bufIn.len’ may be used uninitialized in this function
drivers/message/fusion/mptctl.c:1765: warning: ‘bufOut.len’ 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 <satyam@infradead.org>

---

 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 ++;
 

[PATCH -mm] es18xx: Shut up uninitialized var build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1135 bytes --]


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.

Signed-off-by: Satyam Sharma <satyam@infradead.org>

---

 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);

Re: [PATCH -mm] es18xx: Shut up uninitialized var build warning

[Rene Herman]

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.

[PATCH -mm] drivers/md/: Shut up uninitialized variable warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2098 bytes --]


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(-)

--- 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;
 

Re: [PATCH -mm] drivers/md/: Shut up uninitialized variable warnings

[Jeff Garzik]

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

Re: [PATCH -mm] drivers/md/: Shut up uninitialized variable warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1920 bytes --]



On Sun, 2 Sep 2007, Jeff Garzik wrote:

> 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,

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 ...

[PATCH -mm] DC395x SCSI driver: Shut up uninitialized variable build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 845 bytes --]


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 <satyam@infradead.org>

---

 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;

Re: [PATCH -mm] DC395x SCSI driver: Shut up uninitialized variable build warning

[Jeff Garzik]

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 <satyam@infradead.org>
> 
> ---
> 
>  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

Re: [PATCH -mm] DC395x SCSI driver: Shut up uninitialized variable build warning

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2076 bytes --]



On Sun, 2 Sep 2007, Jeff Garzik wrote:
> 
> 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 <satyam@infradead.org>
> > 
> > ---
> > 
> >  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

Re: [PATCH -mm] DC395x SCSI driver: Shut up uninitialized variable build warning

[Jeff Garzik]

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.

[PATCH -mm] i386 cpuid_count: Fix argument signedness warnings

[Satyam Sharma]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2237 bytes --]


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 ‘cpuid_count’:
include/asm/processor.h:615: warning: pointer targets in passing argument 1 of ‘native_cpuid’ differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 2 of ‘native_cpuid’ differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 3 of ‘native_cpuid’ differ in signedness
include/asm/processor.h:615: warning: pointer targets in passing argument 4 of ‘native_cpuid’ 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 <satyam@infradead.org>

---

 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;