From: Greg KH <gregkh@suse.de>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Valdis.Kletnieks@vt.edu,
"Justin P. Mattock" <justinmattock@gmail.com>,
linux-usb@vger.kernel.org, dbrownell@users.sourceforge.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2]drivers/usb/core/sysfs.c Fix variable 'retval' set but not used
Date: Tue, 3 Aug 2010 10:22:04 -0700 [thread overview]
Message-ID: <20100803172204.GC1123@suse.de> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1008031301490.1853-100000@iolanthe.rowland.org>
On Tue, Aug 03, 2010 at 01:09:31PM -0400, Alan Stern wrote:
> On Tue, 3 Aug 2010, Greg KH wrote:
>
> > On Tue, Aug 03, 2010 at 11:34:26AM -0400, Alan Stern wrote:
> > > On Tue, 3 Aug 2010 Valdis.Kletnieks@vt.edu wrote:
> > >
> > > > > Failure to create a file in sysfs is almost never fatal and usually not
> > > > > even dangerous. Ignoring the error is generally better than failing
> > > > > the entire operation.
> > > >
> > > > Then why the __must_check attribute if it's usually ignorable? I thought
> > > > that was reserved for functions that you damned sight better well check
> > > > for errors because bad things are afoot otherwise?
> > >
> > > That's a good question. Perhaps Greg KH knows the answer.
> >
> > You should check the return value for that function. To not do that is
> > a bug. This one looks like it snuck through. Fixing it properly is the
> > correct thing to do.
>
> In other words, usb_set_configuration() should fail if
> usb_create_sysfs_intf_files() is unable to create the attribute file
> containing an interface's string descriptor?
>
> And likewise, ehci_run() should fail if the "companion" and debugging
> files can't be created?
Ah, yeah, now I recall why we didn't do anything with those values :)
It's best to continue on. Perhaps we should just emit a warning to the
system log if the file fails to be created and move on. That would
properly handle the return value and keep gcc, and everyone else, happy.
thanks,
greg k-h
prev parent reply other threads:[~2010-08-03 17:22 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-03 4:26 [PATCH 1/2]drivers/usb/core/sysfs.c Fix variable 'retval' set but not used Justin P. Mattock
2010-08-03 4:26 ` [PATCH 2/2]drivers/usb/host/ehci-hub.c Fix variable 'i' " Justin P. Mattock
2010-08-03 11:28 ` [PATCH 1/2]drivers/usb/core/sysfs.c Fix variable 'retval' " Valdis.Kletnieks
2010-08-03 13:30 ` Justin P. Mattock
2010-08-03 14:29 ` Alan Stern
2010-08-03 14:43 ` Justin P. Mattock
2010-08-03 15:36 ` Alan Stern
2010-08-03 16:12 ` Justin P. Mattock
2010-08-03 15:13 ` Valdis.Kletnieks
2010-08-03 15:34 ` Alan Stern
2010-08-03 15:46 ` Greg KH
2010-08-03 17:09 ` Alan Stern
2010-08-03 17:22 ` Greg KH [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100803172204.GC1123@suse.de \
--to=gregkh@suse.de \
--cc=Valdis.Kletnieks@vt.edu \
--cc=dbrownell@users.sourceforge.net \
--cc=justinmattock@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox