[PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[Luca Tettamanti]

sysfs_create_bin_file() is marked as warn_unused_result but we don't
actually check the return value.
Error is not fatal, the driver can operate fine without the files so
just print a notice on failure.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>

---

 drivers/video/aty/radeon_base.c |   15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/video/aty/radeon_base.c b/drivers/video/aty/radeon_base.c
index 0ed577e..bc2aac6 100644
--- a/drivers/video/aty/radeon_base.c
+++ b/drivers/video/aty/radeon_base.c
@@ -2313,10 +2313,17 @@ #endif
 	radeon_check_modes(rinfo, mode_option);
 
 	/* Register some sysfs stuff (should be done better) */
-	if (rinfo->mon1_EDID)
-		sysfs_create_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr);
-	if (rinfo->mon2_EDID)
-		sysfs_create_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr);
+	if (rinfo->mon1_EDID) {
+		if (sysfs_create_bin_file(&rinfo->pdev->dev.kobj, &edid1_attr))
+			printk(KERN_INFO "radeonfb (%s): failed to create edid1 file. "
+				"Continuing anyway.\n", pci_name(rinfo->pdev));
+	}
+
+	if (rinfo->mon2_EDID) {
+		if (sysfs_create_bin_file(&rinfo->pdev->dev.kobj, &edid2_attr))
+			printk(KERN_INFO "radeonfb (%s): failed to create edid2 file. "
+				"Continuing anyway.\n", pci_name(rinfo->pdev));
+	}
 
 	/* save current mode regs before we switch into the new one
 	 * so we can restore this upon __exit


Luca
-- 
Dicono che il cane sia il miglior amico dell'uomo. Secondo me non e`
vero. Quanti dei vostri amici avete fatto castrare, recentemente?

Re: [PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[Benjamin Herrenschmidt]

On Thu, 2006-10-12 at 01:53 +0200, Luca Tettamanti wrote:
> sysfs_create_bin_file() is marked as warn_unused_result but we don't
> actually check the return value.
> Error is not fatal, the driver can operate fine without the files so
> just print a notice on failure.

I find this whole business of must check return value for sysfs files to
be gratuitous bloat. There are many cases (like this one) where we don't
really care and a printk will just increase the kernel size for no good
reason.

Maybe we can have a macro we can use to silence the warning when we
don't care about the result ? Can gcc do that ?

Ben.

Re: [PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[Luca Tettamanti]

Il Thu, Oct 12, 2006 at 10:07:26AM +1000, Benjamin Herrenschmidt ha scritto: 
> On Thu, 2006-10-12 at 01:53 +0200, Luca Tettamanti wrote:
> > sysfs_create_bin_file() is marked as warn_unused_result but we don't
> > actually check the return value.
> > Error is not fatal, the driver can operate fine without the files so
> > just print a notice on failure.
> 
> I find this whole business of must check return value for sysfs files to
> be gratuitous bloat. There are many cases (like this one) where we don't
> really care and a printk will just increase the kernel size for no good
> reason.
> 
> Maybe we can have a macro we can use to silence the warning when we
> don't care about the result ? Can gcc do that ?

Ugly macro:

#define UNCHECKED(func) do { if (func) {} } while(0)

maybe it's better to have something like this:

int __sysfs_create_bin_file(...);
inline int sysfs_create_bin_file(...) __attribute__((warn_unused_result));

inline int sysfs_create_bin_file(...) {
        return __sysfs_create_bin_file(...);
}

i.e. both checked and uncheck version of the same function.

Luca
-- 
Un apostolo vedendo Gesu` camminare sulle acque:
- Cazzo se e` buono 'sto fumo!!!

Re: [PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[Andreas Schwab]

Luca Tettamanti <kronos.it@gmail.com> writes:

> Il Thu, Oct 12, 2006 at 10:07:26AM +1000, Benjamin Herrenschmidt ha scritto: 
>> On Thu, 2006-10-12 at 01:53 +0200, Luca Tettamanti wrote:
>> > sysfs_create_bin_file() is marked as warn_unused_result but we don't
>> > actually check the return value.
>> > Error is not fatal, the driver can operate fine without the files so
>> > just print a notice on failure.
>> 
>> I find this whole business of must check return value for sysfs files to
>> be gratuitous bloat. There are many cases (like this one) where we don't
>> really care and a printk will just increase the kernel size for no good
>> reason.
>> 
>> Maybe we can have a macro we can use to silence the warning when we
>> don't care about the result ? Can gcc do that ?
>
> Ugly macro:
>
> #define UNCHECKED(func) do { if (func) {} } while(0)

Better, but only marginally:

#define UNCHECKED(func) (void)(func)

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH 2.6.19-rc1] radeonfb: check return value of sysfs_create_bin_file

[Luca Tettamanti]

Il Thu, Oct 12, 2006 at 05:54:55PM +0200, Andreas Schwab ha scritto: 
> Luca Tettamanti <kronos.it@gmail.com> writes:
> 
> > Il Thu, Oct 12, 2006 at 10:07:26AM +1000, Benjamin Herrenschmidt ha scritto: 
> >> On Thu, 2006-10-12 at 01:53 +0200, Luca Tettamanti wrote:
> >> > sysfs_create_bin_file() is marked as warn_unused_result but we don't
> >> > actually check the return value.
> >> > Error is not fatal, the driver can operate fine without the files so
> >> > just print a notice on failure.
> >> 
> >> I find this whole business of must check return value for sysfs files to
> >> be gratuitous bloat. There are many cases (like this one) where we don't
> >> really care and a printk will just increase the kernel size for no good
> >> reason.
> >> 
> >> Maybe we can have a macro we can use to silence the warning when we
> >> don't care about the result ? Can gcc do that ?
> >
> > Ugly macro:
> >
> > #define UNCHECKED(func) do { if (func) {} } while(0)
> 
> Better, but only marginally:
> 
> #define UNCHECKED(func) (void)(func)

Nope, I tried[1] before sending the mail ;) warn_unused_result requires
that you _use_ the result.

Luca
[1] kronos:~$ gcc --version
gcc (GCC) 4.1.2 20061007 (prerelease) (Debian 4.1.1-16)
-- 
"It is more complicated than you think"
                -- The Eighth Networking Truth from RFC 1925