Re: [PATCH] Silence 'ignoring return value' warnings indrivers/video/aty/radeon_base.c

From: Benjamin Herrenschmidt
Date: Wed May 07 2008 - 00:34:45 EST



On Tue, 2008-05-06 at 18:20 -0700, Andrew Morton wrote:
> Look at the history of this. A couple of years ago we were seeing a huge
> number of mysterious crashes and warnings in and around sysfs and driver
> code and we just didn't have a clue what was causing it, because those
> crashes were happening long, long after the buggy code had done its work.
>
> So I went in there and found a tremendous amount of code in driver core,
> sysfs and in callers of both which was just ignoring error returns and
> blundering on.
>
> It was a comnplete undebuggable unmaintainable mess. And the reason why it
> was undebuggable was because the code was failing to detect errors *when
> they occurred*. So we (me, Cornelia, Greg, others) set about fixing all of
> that. And to support that effort we marked all the things which should be
> checked with __must_check.

You haven't read me properly. I'm not advocating completely ignoring
those errors. In fact, I'm all about keeping must check on things like
allocations. However, in cases like sysfs_create_file() like many
similar things where failure will -not- prevent proper operations of the
driver or subsystem, mostly only compromise the user ABI, I believe it's
a _LOT_ more efficient to put -one- printk in the function itself,
rather than all callers

> Now you come along and cherrypick a few callsites where you'd rather not
> bother checking and assert that the entire effort was wrong-headed. Well
> sorry, no, it wasn't. Sure, there's a little bit of undesirable fallout
> but the whole thing had. to. be. done.

Of course the whole effort was not wrong headed. I'm really only
complaining about all those stupid sysfs_create_file() and maybe a
handful of similar ones.

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/