On Wednesday 23 July 2008, Henrique de Moraes Holschuh wrote:
> On Wed, 23 Jul 2008, Johannes Berg wrote:
> > On Wed, 2008-07-23 at 12:27 -0300, Henrique de Moraes Holschuh wrote:
> > > On Wed, 23 Jul 2008, Johannes Berg wrote:
> > > > > + list_for_each_entry(p, &rfkill_list, node) {
> > > > > + if (p == rfkill)
> > > > > + return -EEXIST;
> > > > > + set_bit(p->type, &seen);
> > > >
> > > > You should WARN_ON so it can get fixed.
> > >
> > > I'd rather make rfkill_register __must_check, actually. With or without
> > > WARN_ON.
> > >
> > > In fact, lots of stuff in rfkill should be __must_check. Ivo, what do you
> > > think? Add __must_check? Add WARN_ON? Add both?
> >
> > Do both then. __must_check annotations get ignored regularly, or
> > "handled" like this:
> >
> > if (...)
> > // nothing we can do.
>
> Ah, that gives you heavy spiked LART rights to the head of whomever does it
> if he is wrong about it. You should fail to load the module or something
> like that if you are going to run half-broken without rfkill support.
>
> I will add the WARN_ON. __must_check will wait for Ivo's reply, as you
> said, it is often ignored.
I usually dislike WARN_ON, but like Johannes said, this one is a bug in the driver
which must be fixed. When you submit the final version of the patch having
the WARN_ON would be best.
I am however a fan on sparse annotation and using __must_check, but since you
already stated that more functions might need it, it doesn't have to be in this patch
and could wait for a later patch. (But it is fine if you want to add it in this patch as well,
off course. ;) )
Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
