On Thu, Jul 3, 2008 at 7:47 PM, Ivo van Doorn <ivdoorn@xxxxxxxxx> wrote:
> On Thursday 03 July 2008, Tomas Winkler wrote:
>> On Thu, Jul 3, 2008 at 7:14 PM, Henrique de Moraes Holschuh
>> <hmh@xxxxxxxxxx> wrote:
>> > rfkill_add_switch() calls rfkill_toggle_radio() to set the state of a
>> > recently registered rfkill class to the current global state [for that
>> > rfkill->type].
>> >
>> > The rfkill_toggle_radio() call is going to error out if the hardware is
>> > RFKILL_STATE_HARD_BLOCKED, and the global state is RFKILL_STATE_UNBLOCKED.
>> >
>> > That is a quite normal situation which I missed to account for. As things
>> > stand, the error return from rfkill_toggle_radio ends up causing
>> > rfkill_register to bail out with an error (de-registering the new switch in
>> > the process), which is Not Nice.
>> >
>> > Change rfkill_add_switch() to not return errors because of a failed call to
>> > rfkill_toggle_radio(). We can go back to returning errors again (if that's
>> > indeed the right thing to do) if we define the exact error codes the
>> > rfkill->toggle_radio callbacks are to return in each situation, so that we
>> > can ignore the right ones only.
>> >
>> > Bug reported by "kionez <kionez@xxxxxxxx>".
>> >
>> > Signed-off-by: Henrique de Moraes Holschuh <hmh@xxxxxxxxxx>
>> > Cc: Ivo van Doorn <IvDoorn@xxxxxxxxx>
>> > Cc: kionez <kionez@xxxxxxxx>
>> > ---
>> > net/rfkill/rfkill.c | 10 ++++------
>> > 1 files changed, 4 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
>> > index aa7039d..7a560b7 100644
>> > --- a/net/rfkill/rfkill.c
>> > +++ b/net/rfkill/rfkill.c
>> > @@ -501,17 +501,15 @@ static struct class rfkill_class = {
>> >
>> > static int rfkill_add_switch(struct rfkill *rfkill)
>> > {
>> > - int error;
>> > -
>> > mutex_lock(&rfkill_mutex);
>> >
>> > - error = rfkill_toggle_radio(rfkill, rfkill_states[rfkill->type], 0);
>> > - if (!error)
>> > - list_add_tail(&rfkill->node, &rfkill_list);
>> > + rfkill_toggle_radio(rfkill, rfkill_states[rfkill->type], 0);
>> > +
>> > + list_add_tail(&rfkill->node, &rfkill_list);
>> >
>> > mutex_unlock(&rfkill_mutex);
>> >
>> > - return error;
>> > + return 0;
>> > }
>>
>> So why this is not a void function
>
> Well as Henrique suggested we might add a correct return value later when
> we figured out which error codes should be returned. If the interface now goes
> to a void function, we need to fix all drivers only later to revert those changes
> again when it returns an int again.
> And this way drivers will at least keep it mind that the function might fail,
> perhaps not now, but perhaps later, when for example we are sane-checking
> the filled in fields of the rfkill structure, or do some other fancy things which
> might fail.
Fair enough
Tomas
--
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
