On Mon, 9 Aug 2010, Gustavo F. Padovan wrote:
Hi Marcel,
* Marcel Holtmann <marcel@xxxxxxxxxxxx> [2010-08-08 22:43:00 -0400]:
Hi Mat,
This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16
with L2CAP modes other than ERTM or streaming. At present, this only
affects the FCS value shown with getsockopt() for basic mode.
Signed-off-by: Mat Martineau <mathewm@xxxxxxxxxxxxxx>
---
net/bluetooth/l2cap.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 3e3cd9d..bc10be8 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3072,6 +3072,16 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
return 0;
}
+static inline int l2cap_fcs_needed(struct l2cap_pinfo *pi)
+{
+ if (pi->mode != L2CAP_MODE_ERTM && pi->mode != L2CAP_MODE_STREAMING)
+ return 0;
+
+ /* FCS is enabled if one or both sides request it. */
+ return !(pi->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
+ pi->fcs == L2CAP_FCS_CRC16;
+}
so I think a switch statement is easier to read:
switch (pi->mode) {
case L2CAP_MODE_ERTM:
case L2CAP_MODE_STREAMING:
return !(pi->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
pi->fcs == L2CAP_FCS_CRC16;
}
return 0;
static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data)
{
struct l2cap_conf_req *req = (struct l2cap_conf_req *) data;
@@ -3136,8 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr
goto unlock;
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
- if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) ||
- l2cap_pi(sk)->fcs != L2CAP_FCS_NONE)
+ if (l2cap_fcs_needed(l2cap_pi(sk)))
l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16;
While this is just fine, I think we might should make this a lot
simpler:
if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) {
set_default_fcs(pi);
...
}
With then set_default_fcs() doing all the checks etc. and setting either
FCS_NONE or FCS_CRC16.
Would this work or do we have to deal with some funny default behavior?
That will work. ;)
Agreed - that's much more readable.
--
Mat Martineau
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
--
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
