drivers/usb/core/sysfs.c Fix variable 'retval' set but not used
in [Kernel]
|
Prev: [PATCH 1/9] v4 Move the find_memory_block() routine up
Next: [2.6.36, patch] unprotected access to task credentials in waitid()
From: Justin P. Mattock on 3 Aug 2010 12:20 On 08/03/2010 08:36 AM, Alan Stern wrote: > On Tue, 3 Aug 2010, Justin P. Mattock wrote: > >> On 08/03/2010 07:29 AM, Alan Stern wrote: >>> On Tue, 3 Aug 2010 Valdis.Kletnieks(a)vt.edu wrote: >>> >>>> On Mon, 02 Aug 2010 21:26:28 PDT, "Justin P. Mattock" said: >>>>> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c >>>> >>>>> if (alt->string) >>>>> - retval = device_create_file(&intf->dev,&dev_attr_interface); >>>>> + device_create_file(&intf->dev,&dev_attr_interface); >>>>> intf->sysfs_files_created = 1; >>>>> return 0; >>> >>> Justin, did you try compiling your new code? Those unused values are >>> there because device_create_file is declared as __must_check. >>> >> >> I went as far as compiling, once I saw no warning then figured o.k >> I'll send out what I have for feedback then go from there. >> (and just for the record I want to thank those who took the time to go >> through and give feedback). > > It's a little surprising that you didn't get any warning. I guess you > don't have CONFIG_ENABLE_MUST_CHECK turned on. > > Alan Stern > > no nothing.. just the original warning.. as for MUST_CHECK yeah your right it's off. Justin P. Mattock -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Alan Stern on 3 Aug 2010 13:10 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(a)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? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 3 Aug 2010 13:30
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(a)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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |