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

[Cornelia Huck]

On Wed, 07 May 2008 14:33:24 +1000,
Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx> wrote:

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

But they are often an indication that we messed up earlier (e. g. try
to add something twice)...

> mostly only compromise the user ABI, 

Which is bad enough in itself. Most people will want to avoid a
crippled 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.

Hm, just took a look at the code:

- for "entry already exists", sysfs will already spit a warning.
- for "argh, we can't get a dirent", sysfs won't say anything.

The first one is the one we really want to yell about, since we've
messed up somewhere. The second one is not as likely, maybe we want to
warn about it when we activate debug options?

Which of the current __must_check functions do you think should have
the __must_check removed?
--
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/