Hi Jaganath, On Fri, Jul 06, 2012, Jaganath Kanakkassery wrote: > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c > index 5a442b9..4fc3379 100644 > --- a/net/bluetooth/mgmt.c > +++ b/net/bluetooth/mgmt.c > @@ -1764,6 +1764,10 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status) > bacpy(&rp.addr.bdaddr, &conn->dst); > rp.addr.type = link_to_bdaddr(conn->type, conn->dst_type); > > + /* Override status if local device rejected pairing */ > + if (conn->auth_rejected == true) > + status = MGMT_STATUS_REJECTED; I think simply "if (conn->auth_rejected)" should be fine (no "== true"). And what if status == 0 and this is a repairing over the same hci_conn which was previously rejected? Seems like you'd give a false negative in that case. Maybe the check should be "if (status && conn->auth_rejected)". > + /* Override status if local device rejected pairing */ > + if (auth_rejected == true) Same thing again with the comparison. The stuff inside () of an if-statement should be a valid boolean, and if the standard bool type by itself can't be considered that then I don't know what can. The thing that's worrying me is that there's nowhere where you clear conn->auth_rejected. If a re-authentication is attempted with the same hci_conn the code would be doing wrong things. I'm not completely sure where this clearing should occur since we're potentially sending two mgmt events through two different code paths (pairing_complete and mgmt_auth_failed) so clearing in either one might be risky in that it causes the second function to do the wrong thing. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
