* Re: [PATCH 2/3] Remove ifndef barrier from log.h and btio.h
From: Johan Hedberg @ 2010-06-08 6:19 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: linux-bluetooth
In-Reply-To: <1275712886-20127-2-git-send-email-gustavo@padovan.org>
Hi Gustavo,
On Sat, Jun 05, 2010, Gustavo F. Padovan wrote:
> ---
> src/btio.h | 3 ---
> src/log.h | 4 +---
> 2 files changed, 1 insertions(+), 6 deletions(-)
>
> diff --git a/src/btio.h b/src/btio.h
> index 00d743e..fa6ff69 100644
> --- a/src/btio.h
> +++ b/src/btio.h
> @@ -21,8 +21,6 @@
> * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> *
> */
> -#ifndef BT_IO_H
> -#define BT_IO_H
I think it'd be good to leave this for BtIO since it might become a
public library soon.
> -#ifndef __LOGGING_H
> -#define __LOGGING_H
> +extern int debug_enabled;
This doesn't match the commit message. To me it seems like the extern
int addition should be part of the third patch and not this one. Also,
Marcel might have had a reason for putting the safeguard for log.h so
just leave these safeguards as they are (ofono and connman has them
too).
Johan
^ permalink raw reply
* Re: [PATCHv1 0/2] Don't delete l2cap chan and sk when sk is owned by user
From: Andrei Emeltchenko @ 2010-06-07 14:11 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1274883694-18120-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
On Wed, May 26, 2010 at 5:21 PM, Emeltchenko Andrei
<Andrei.Emeltchenko.news@gmail.com> wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>
> When sk is owned by user we shall not delete l2cap shannel structures and socket sk.
> The first patch which is less hackish than using timer to delete l2cap channel.
>
> Andrei Emeltchenko (2):
> Bluetooth: Check sk is not owned before freeing l2cap_conn
> Bluetooth: Prevent sk freeing in tasklet using refcount
>
> net/bluetooth/l2cap.c | 16 +++++++++++++---
> 1 files changed, 13 insertions(+), 3 deletions(-)
Ping
^ permalink raw reply
* Re: [PATCH v2] frame reassembly implementation for data stream
From: Suraj @ 2010-06-07 4:17 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Suraj Sumangala, linux-bluetooth@vger.kernel.org, Luis Rodriguez,
Jothikumar Mothilal
In-Reply-To: <4C0754CD.7020902@Atheros.com>
Hi Marcel,
On 6/3/2010 12:37 PM, Suraj wrote:
> Hi Marcel,
>
> On 6/3/2010 12:08 PM, Marcel Holtmann wrote:
>> Hi Suraj,
>>
>>>> I don't like this implementation at all. The biggest problem is that you
>>>> are misusing __reassembly(hdev, HCI_ACLDATA_PKT) for getting your SKB. I
>>>> don't wanna intermix this. I am also missing checks for the packet
>>>> length matching or when packets are too big or the header size is not
>>>> matching up.
>>>>
>>>> So in theory both functions do exactly the same. Only minor exception is
>>>> that one knows the packet type up-front, the other has to read it from
>>>> the stream as a 1-byte header. I don't wanna maintain two functions that
>>>> do exactly the same.
>>>>
>>>> Creating an internal helper function that can maintain the current state
>>>> of the reassembly sounds a lot better. Then re-use that function and
>>>> ensure that the reassembly logic is inside the helper.
>>>
>>> I appreciate if you can take a closer look at the patch and compare it
>>> with hci_recv_fragment implementation.
>>> Eventhough it looks similar, there are major differences on the way data
>>> is reassembled. It would not have worked if I had reused the same code
>>> from hci_recv_fragment().
>>>
>>> Having a common reassembly helper could work. But I am not sure whether
>>> that would be a better solution.
>>
>> I did have a closer look at it already. I clearly see possibilities to
>> combine them into a more generic helper and not to maintain two
>> different functions that do exactly the same.
>
> The cases that the reassembly code has to consider are
>
> 1. We receive only packet type in one fragment
> 2. packet header is fragmented
> 3. packet data is fragmented
> 4. We receive multiple frames in one fragment.
>
> hci_recv_fragment(), handles only case 3 and 4 as it always knows the
> packet type and the full packet header in one call itself.
>
> For stream reassembly we need to take care of all cases 1-4 . This is
> where both functions differ.
>
> This actually makes a lot of difference in the way reassembly is done.
>
> Now regarding having a common helper function. I agree to the fact that
> it makes sense to have a helper function.
>
> My points against a common helper function are
>
> 1. It has to cover all 1-4 cases. This make it bit inefficient for USB
> as not considering 1 and 2 saves a lot of effort( allocating buffer
> before knowing the length, trying to verify if we have received the
> packet header or not etc etc.).
>
> 2. For case 4: the helper will not know from which context it was called.
> If it was called from a stream context, it will have to take the first
> byte of the next frame as packet type and do reassembly accordingly.
> This involves the caller telling the helper function from what context
> it was called. Which IMHO makes it less generic
>
> I understand that these issues can be worked around, but I believe makes
> the generic implementation kind of "trying to do too many things" helper.
>
Do let me know if you still think that having a generic function makes
more sense, then I can go ahead and implement it.
Regards
Suraj
^ permalink raw reply
* [PATCH] Get IEEE1284 for a single printer
From: Bastien Nocera @ 2010-06-06 18:42 UTC (permalink / raw)
To: linux-bluetooth
[-- Attachment #1: Type: text/plain, Size: 714 bytes --]
Heya,
Discussed printer setup integration with Tim Waugh, and we came to the
idea that the current framework for USB printers could be reused for
Bluetooth ones.
The differences would be:
- add code to fetch a single printer's IEEE1284 device ID to the
Bluetooth cups backend (the attached patch)
- add special handling in udev-configure-printer for Bluetooth printers
(we get the IEEE1284 device ID from the cups backend helper)
https://fedorahosted.org/system-config-printer/ticket/207
- Add plugin to bluetoothd to call udev-configure-printer when printers
are paired, or unpaired (not written yet)
Let me know what you think. The patch below could still be useful for
debugging (in the worst case).
Cheers
[-- Attachment #2: 0001-cups-Add-ability-to-print-IEEE1284-device-ID.patch --]
[-- Type: text/x-patch, Size: 3209 bytes --]
>From da8e0589d687cb3f24eab46259d179fd830cd088 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Sun, 6 Jun 2010 15:48:26 +0100
Subject: [PATCH] (cups) Add ability to print IEEE1284 device ID
Add ability to print IEEE1284 device ID for Bluetooth
printers to allow auto-configuration once paired.
---
cups/main.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 78 insertions(+), 0 deletions(-)
diff --git a/cups/main.c b/cups/main.c
index 9659a11..00933b6 100644
--- a/cups/main.c
+++ b/cups/main.c
@@ -605,6 +605,75 @@ static gboolean list_printers(void)
return TRUE;
}
+static gboolean print_ieee1284(const char *bdaddr)
+{
+ DBusMessage *message, *reply, *adapter_reply;
+ DBusMessageIter iter;
+ char *object_path = NULL;
+ char *adapter;
+ char *id;
+
+ adapter_reply = NULL;
+
+ conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
+ if (conn == NULL)
+ return FALSE;
+
+ message = dbus_message_new_method_call("org.bluez", "/",
+ "org.bluez.Manager",
+ "DefaultAdapter");
+
+ adapter_reply = dbus_connection_send_with_reply_and_block(conn,
+ message, -1, NULL);
+
+ dbus_message_unref(message);
+
+ if (dbus_message_get_args(adapter_reply, NULL, DBUS_TYPE_OBJECT_PATH, &adapter, DBUS_TYPE_INVALID) == FALSE)
+ return FALSE;
+
+ message = dbus_message_new_method_call("org.bluez", adapter,
+ "org.bluez.Adapter",
+ "FindDevice");
+ dbus_message_iter_init_append(message, &iter);
+ dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
+
+ if (adapter_reply != NULL)
+ dbus_message_unref(adapter_reply);
+
+ reply = dbus_connection_send_with_reply_and_block(conn,
+ message, -1, NULL);
+
+ dbus_message_unref(message);
+
+ if (!reply) {
+ message = dbus_message_new_method_call("org.bluez", adapter,
+ "org.bluez.Adapter",
+ "CreateDevice");
+ dbus_message_iter_init_append(message, &iter);
+ dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
+
+ reply = dbus_connection_send_with_reply_and_block(conn,
+ message, -1, NULL);
+
+ dbus_message_unref(message);
+
+ if (!reply)
+ return FALSE;
+ }
+ if (dbus_message_get_args(reply, NULL, DBUS_TYPE_OBJECT_PATH, &object_path,
+ DBUS_TYPE_INVALID) == FALSE) {
+ return FALSE;
+ }
+
+ id = device_get_ieee1284_id(adapter, object_path);
+ if (id == NULL)
+ return FALSE;
+ printf("%s", id);
+ g_free(id);
+
+ return TRUE;
+}
+
/*
* Usage: printer-uri job-id user title copies options [file]
*
@@ -642,10 +711,19 @@ int main(int argc, char *argv[])
return CUPS_BACKEND_OK;
else
return CUPS_BACKEND_FAILED;
+ } else if (argc == 3 && strcmp(argv[1], "--get-deviceid") == 0) {
+ if (bachk(argv[2]) < 0) {
+ fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
+ return CUPS_BACKEND_FAILED;
+ }
+ if (print_ieee1284(argv[2]) == FALSE)
+ return CUPS_BACKEND_FAILED;
+ return CUPS_BACKEND_OK;
}
if (argc < 6 || argc > 7) {
fprintf(stderr, "Usage: bluetooth job-id user title copies options [file]\n");
+ fprintf(stderr, " bluetooth --get-deviceid [bdaddr]\n");
return CUPS_BACKEND_FAILED;
}
--
1.7.0.1
^ permalink raw reply related
* Re: Inquiry Result with repeated set of MAC addresses - Is it a problem with Bluetooth device ?
From: Manuel Naranjo @ 2010-06-06 18:04 UTC (permalink / raw)
To: Saurabh Thukral; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimX-ofkv4oekES0kfi7l_y4yLMVAO8A9e0Nx5tg@mail.gmail.com>
Hi,
> On running <hcitool scan> in one terminal and simultaneously viewing
> results using <hcidump> on the other terminal, I had a very
> interesting observation that has made me quite perplexed. I would
> really like you to help me out.
>r
> The observation is that suppose a device(Nokia N73) with MAC address
> M1 is discoverable, the response to Inquiry Request command is often a
> set of multiple Inquiry Result each with the same MAC address M1.
This is totally normal! It's related to how bluetooth inquiry process
works. The device doing the inquiry is jumping at a rate of 1600
hops/s (If I'm not wrong) over 32 channels and te device in inquiry
scan is jumpung at a way lower rate. So the radio might, and actually
will, find the same device multiple times per scan cycle. hcidump
shows you exactly which information is going on between the radio and
the stack.
The advantage of getting multiple results are many:
* RSSI depends on scanning channel: not all the channel have the same
noise level, so averaging RSSI with multiple results is way more
accurate than just one rssi value from one only channel.
* Faster response: hcitool inq will only show you the results once it
has solved both name and completed scanning, but actually it's getting
the results way before. I had been able to expermient dicovering a new
radio in less than 1 second so I could take action, and this can only
be done if you get multiple results per scan cycle.
Off course it requires more inteligence on you side. You need to
filter which results are new and which are not, but that is damn easy,
and you would be loose applications if it was otherwise.
Manuel
--
Manuel Francisco Naranjo
Software Department Argentina
Wireless Cables Inc
www.aircable.net
cel: +5493412010019
skype: naranjomanuelfrancisco
^ permalink raw reply
* Re: Inquiry Result with repeated set of MAC addresses - Is it a problem with Bluetooth device ?
From: Johan Hedberg @ 2010-06-06 15:09 UTC (permalink / raw)
To: Saurabh Thukral; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimcKrBh6kVCo1mtZgqxQkPip8xg8ADQhEuFJc82@mail.gmail.com>
Hi,
On Sun, Jun 06, 2010, Saurabh Thukral wrote:
> This behaviour is observed on HP Elitebook using broadcom bluetooth
> adaptor. However when I run the same test on Compaq 510 laptop having
> the same adaptor, only a single Inquiry result is returned and then
> inquiry completes.
Is there any difference between the host operating systems and bluetooth
stacks on these two laptops?
> Please note that this is irrespective of whether there is standard
> inquiry or Inquiry with RSSI.
If you get the behavior for non-RSSI events it sounds like a bug in the
controller.
> I think having multiple inquiry results
> all having same MAC address is wrong for following 2 reasons:
>
> 1) The procedure of discovery of devices becomes longer
>
> 2) I programmed getting inquiry result by sending hci_send_cmd and
> then polling on hci socket fd for all the events. Since I would be
> getting multiple events all having result for the same MAC address,
> this creates a burden to weed out all duplicate entries and return one
> entry containing MAC address only. I think this way is wrong.
You're entitled to your opinion and I do understand it to some extent,
but unfortunately I'm not aware of any standard HCI commands or
parameters through which you could make the controller not exhibit this
behavior (the behavior with non-RSSI events seems like a bug in the
controller though). Btw, why not let bluetoothd take care of the
discovery for you? It already has code for detecting duplicate BT
addresses (though it will send multiple D-Bus signals for them in case a
UI is interested in the signal strength variations).
Johan
^ permalink raw reply
* Re: Inquiry Result with repeated set of MAC addresses - Is it a problem with Bluetooth device ?
From: Saurabh Thukral @ 2010-06-06 14:06 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <20100606085624.GA393@jh-x301>
Saurabh Wrote:
>> The observation is that suppose a device(Nokia N73) with MAC address
>> M1 is discoverable, the response to Inquiry Request command is often a
>> set of multiple Inquiry Result each with the same MAC address M1.
>>
>> I had expected that only one Inquiry result with MAC address M1 should
>> have been returned. But here what we find is that multiple such
>> results with the same MAC address are returned.
>>
Johan Wrote:
>
> It's most likely neither, but simply as it's intended to be. I presume
> the inquiry results contain the RSSI which will most likely be different
> in each event. The purpose of the multiple events is to give you updates
> if the signal strength to the device changes.
This behaviour is observed on HP Elitebook using broadcom bluetooth
adaptor. However when I run the same test on Compaq 510 laptop having
the same adaptor, only a single Inquiry result is returned and then
inquiry completes.
Please note that this is irrespective of whether there is standard
inquiry or Inquiry with RSSI. I think having multiple inquiry results
all having same MAC address is wrong for following 2 reasons:
1) The procedure of discovery of devices becomes longer
2) I programmed getting inquiry result by sending hci_send_cmd and
then polling on hci socket fd for all the events. Since I would be
getting multiple events all having result for the same MAC address,
this creates a burden to weed out all duplicate entries and return one
entry containing MAC address only. I think this way is wrong.
--
Regards,
Saurabh Thukral
^ permalink raw reply
* Re: Inquiry Result with repeated set of MAC addresses - Is it a problem with Bluetooth device ?
From: Johan Hedberg @ 2010-06-06 8:56 UTC (permalink / raw)
To: Saurabh Thukral; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimX-ofkv4oekES0kfi7l_y4yLMVAO8A9e0Nx5tg@mail.gmail.com>
Hi,
On Sun, Jun 06, 2010, Saurabh Thukral wrote:
> I am currently testing my bluetooth application using Bluez on Linux
> (Fedora Core 11) and have HP Eltebook laptop that has Broadcom
> bluetooth adaptor. The Bluez version installed is 4.37.
>
> On running <hcitool scan> in one terminal and simultaneously viewing
> results using <hcidump> on the other terminal, I had a very
> interesting observation that has made me quite perplexed. I would
> really like you to help me out.
>
> The observation is that suppose a device(Nokia N73) with MAC address
> M1 is discoverable, the response to Inquiry Request command is often a
> set of multiple Inquiry Result each with the same MAC address M1.
>
> I had expected that only one Inquiry result with MAC address M1 should
> have been returned. But here what we find is that multiple such
> results with the same MAC address are returned.
>
> Please advise if this is an issue with bluetooth adaptor or bluez.
It's most likely neither, but simply as it's intended to be. I presume
the inquiry results contain the RSSI which will most likely be different
in each event. The purpose of the multiple events is to give you updates
if the signal strength to the device changes.
Johan
^ permalink raw reply
* Inquiry Result with repeated set of MAC addresses - Is it a problem with Bluetooth device ?
From: Saurabh Thukral @ 2010-06-06 5:12 UTC (permalink / raw)
To: linux-bluetooth
Dear Bluez Team,
I am currently testing my bluetooth application using Bluez on Linux
(Fedora Core 11) and have HP Eltebook laptop that has Broadcom
bluetooth adaptor. The Bluez version installed is 4.37.
On running <hcitool scan> in one terminal and simultaneously viewing
results using <hcidump> on the other terminal, I had a very
interesting observation that has made me quite perplexed. I would
really like you to help me out.
The observation is that suppose a device(Nokia N73) with MAC address
M1 is discoverable, the response to Inquiry Request command is often a
set of multiple Inquiry Result each with the same MAC address M1.
I had expected that only one Inquiry result with MAC address M1 should
have been returned. But here what we find is that multiple such
results with the same MAC address are returned.
Please advise if this is an issue with bluetooth adaptor or bluez.
Kindly suggest some workaround.
--
Regards,
Saurabh Thukral
^ permalink raw reply
* [PATCH] audio: Fix typo
From: Gustavo F. Padovan @ 2010-06-06 2:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
---
audio/gateway.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/audio/gateway.c b/audio/gateway.c
index dfe7145..1dd3f5b 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -204,7 +204,7 @@ static void newconnection_reply(DBusPendingCall *call, void *data)
dbus_error_init(&derr);
if (!dbus_set_error_from_message(&derr, reply)) {
- DBG("Agent reply: file descriptor passed successfuly");
+ DBG("Agent reply: file descriptor passed successfully");
change_state(dev, GATEWAY_STATE_CONNECTED);
goto done;
}
@@ -234,7 +234,7 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *err,
}
if (!gw->agent) {
- error("Handfree Agent not registered");
+ error("Handsfree Agent not registered");
goto fail;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 3/3] sdptool: Fix 2 possible NULL dereference
From: Gustavo F. Padovan @ 2010-06-05 10:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275732870-15889-3-git-send-email-gustavo@padovan.org>
Issues found by smatch static check:
http://smatch.sourceforge.net/
---
tools/sdptool.c | 19 +++++++++----------
1 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/tools/sdptool.c b/tools/sdptool.c
index 6fb2e90..89a497a 100644
--- a/tools/sdptool.c
+++ b/tools/sdptool.c
@@ -513,14 +513,16 @@ static void sdp_data_printf(sdp_data_t *sdpdata, struct attrib_context *context,
*/
static void print_tree_attr_func(void *value, void *userData)
{
- sdp_data_t *sdpdata = NULL;
+ sdp_data_t *sdpdata = value;
uint16_t attrId;
struct service_context *service = (struct service_context *) userData;
struct attrib_context context;
struct attrib_def *attrDef = NULL;
int i;
- sdpdata = (sdp_data_t *)value;
+ if (!sdpdata)
+ return;
+
attrId = sdpdata->attrId;
/* Search amongst the generic attributes */
for (i = 0; i < attrib_max; i++)
@@ -549,10 +551,7 @@ static void print_tree_attr_func(void *value, void *userData)
context.attrib = attrDef;
context.member_index = 0;
/* Parse attribute members */
- if (sdpdata)
- sdp_data_printf(sdpdata, &context, 2);
- else
- printf(" NULL value\n");
+ sdp_data_printf(sdpdata, &context, 2);
/* Update service */
service->service = context.service;
}
@@ -723,6 +722,9 @@ static void print_raw_attr_func(void *value, void *userData)
struct attrib_def *def = NULL;
int i;
+ if (!data)
+ return;
+
/* Search amongst the generic attributes */
for (i = 0; i < attrib_max; i++)
if (attrib_names[i].num == data->attrId) {
@@ -735,10 +737,7 @@ static void print_raw_attr_func(void *value, void *userData)
else
printf("\tAttribute 0x%04x\n", data->attrId);
- if (data)
- print_raw_data(data, 2);
- else
- printf(" NULL value\n");
+ print_raw_data(data, 2);
}
static void print_raw_attr(sdp_record_t *rec)
--
1.7.1
^ permalink raw reply related
* [PATCH 2/3] Fix redundant null check on calling free()
From: Gustavo F. Padovan @ 2010-06-05 10:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275732870-15889-2-git-send-email-gustavo@padovan.org>
Issues found by smatch static check: http://smatch.sourceforge.net/
---
input/device.c | 6 +--
lib/sdp.c | 89 +++++++++++++++++----------------------------------
src/sdp-xml.c | 8 +---
src/sdpd-request.c | 12 ++-----
src/storage.c | 6 +--
test/ipctest.c | 6 +--
6 files changed, 42 insertions(+), 85 deletions(-)
diff --git a/input/device.c b/input/device.c
index 8daf8b2..f06b229 100644
--- a/input/device.c
+++ b/input/device.c
@@ -596,8 +596,7 @@ failed:
close(req->ctrl_sock);
cleanup:
- if (req->rd_data)
- free(req->rd_data);
+ free(req->rd_data);
g_free(req);
}
@@ -662,8 +661,7 @@ static int hidp_add_connection(const struct input_device *idev,
err = ioctl_connadd(req);
cleanup:
- if (req->rd_data)
- free(req->rd_data);
+ free(req->rd_data);
g_free(req);
return err;
diff --git a/lib/sdp.c b/lib/sdp.c
index ca3b4c4..79f4ae5 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2915,11 +2915,8 @@ int sdp_device_record_register_binary(sdp_session_t *session, bdaddr_t *device,
}
end:
- if (req)
- free(req);
-
- if (rsp)
- free(rsp);
+ free(req);
+ free(rsp);
return status;
}
@@ -3025,11 +3022,8 @@ int sdp_device_record_unregister_binary(sdp_session_t *session, bdaddr_t *device
status = -1;
}
end:
- if (reqbuf)
- free(reqbuf);
-
- if (rspbuf)
- free(rspbuf);
+ free(reqbuf);
+ free(rspbuf);
return status;
}
@@ -3133,10 +3127,8 @@ int sdp_device_record_update(sdp_session_t *session, bdaddr_t *device, const sdp
status = -1;
}
end:
- if (reqbuf)
- free(reqbuf);
- if (rspbuf)
- free(rspbuf);
+ free(reqbuf);
+ free(rspbuf);
return status;
}
@@ -3472,10 +3464,8 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
} while (cstate);
end:
- if (reqbuf)
- free(reqbuf);
- if (rspbuf)
- free(rspbuf);
+ free(reqbuf);
+ free(rspbuf);
return status;
}
@@ -3649,12 +3639,9 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
}
end:
- if (reqbuf)
- free(reqbuf);
- if (rsp_concat_buf.data)
- free(rsp_concat_buf.data);
- if (rspbuf)
- free(rspbuf);
+ free(reqbuf);
+ free(rsp_concat_buf.data);
+ free(rspbuf);
return rec;
}
@@ -3777,9 +3764,8 @@ int sdp_service_search_async(sdp_session_t *session, const sdp_list_t *search, u
t = session->priv;
- /* check if the buffer is already allocated */
- if (t->rsp_concat_buf.data)
- free(t->rsp_concat_buf.data);
+ /* clean possible allocated buffer */
+ free(t->rsp_concat_buf.data);
memset(&t->rsp_concat_buf, 0, sizeof(sdp_buf_t));
if (!t->reqbuf) {
@@ -3825,10 +3811,8 @@ int sdp_service_search_async(sdp_session_t *session, const sdp_list_t *search, u
return 0;
end:
- if (t->reqbuf) {
- free(t->reqbuf);
- t->reqbuf = NULL;
- }
+ free(t->reqbuf);
+ t->reqbuf = NULL;
return -1;
}
@@ -3881,9 +3865,8 @@ int sdp_service_attr_async(sdp_session_t *session, uint32_t handle, sdp_attrreq_
t = session->priv;
- /* check if the buffer is already allocated */
- if (t->rsp_concat_buf.data)
- free(t->rsp_concat_buf.data);
+ /* clean possible allocated buffer */
+ free(t->rsp_concat_buf.data);
memset(&t->rsp_concat_buf, 0, sizeof(sdp_buf_t));
if (!t->reqbuf) {
@@ -3939,10 +3922,8 @@ int sdp_service_attr_async(sdp_session_t *session, uint32_t handle, sdp_attrreq_
return 0;
end:
- if (t->reqbuf) {
- free(t->reqbuf);
- t->reqbuf = NULL;
- }
+ free(t->reqbuf);
+ t->reqbuf = NULL;
return -1;
}
@@ -3996,9 +3977,8 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
t = session->priv;
- /* check if the buffer is already allocated */
- if (t->rsp_concat_buf.data)
- free(t->rsp_concat_buf.data);
+ /* clean possible allocated buffer */
+ free(t->rsp_concat_buf.data);
memset(&t->rsp_concat_buf, 0, sizeof(sdp_buf_t));
if (!t->reqbuf) {
@@ -4058,10 +4038,8 @@ int sdp_service_search_attr_async(sdp_session_t *session, const sdp_list_t *sear
return 0;
end:
- if (t->reqbuf) {
- free(t->reqbuf);
- t->reqbuf = NULL;
- }
+ free(t->reqbuf);
+ t->reqbuf = NULL;
return -1;
}
@@ -4286,8 +4264,7 @@ end:
t->cb(pdu_id, status, pdata, size, t->udata);
}
- if (rspbuf)
- free(rspbuf);
+ free(rspbuf);
return err;
}
@@ -4521,12 +4498,9 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
}
}
end:
- if (rsp_concat_buf.data)
- free(rsp_concat_buf.data);
- if (reqbuf)
- free(reqbuf);
- if (rspbuf)
- free(rspbuf);
+ free(rsp_concat_buf.data);
+ free(reqbuf);
+ free(rspbuf);
return status;
}
@@ -4557,11 +4531,9 @@ int sdp_close(sdp_session_t *session)
t = session->priv;
if (t) {
- if (t->reqbuf)
- free(t->reqbuf);
+ free(t->reqbuf);
- if (t->rsp_concat_buf.data)
- free(t->rsp_concat_buf.data);
+ free(t->rsp_concat_buf.data);
free(t);
}
@@ -4668,8 +4640,7 @@ fail:
err = errno;
if (session->sock >= 0)
close(session->sock);
- if (session->priv)
- free(session->priv);
+ free(session->priv);
free(session);
errno = err;
diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index fbe608c..4b0520d 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -728,12 +728,8 @@ void sdp_xml_data_free(struct sdp_xml_data *elem)
if (elem->data)
sdp_data_free(elem->data);
- if (elem->name)
- free(elem->name);
-
- if (elem->text)
-
- free(elem->text);
+ free(elem->name);
+ free(elem->text);
free(elem);
}
diff --git a/src/sdpd-request.c b/src/sdpd-request.c
index 8c88d6e..d56ffc2 100644
--- a/src/sdpd-request.c
+++ b/src/sdpd-request.c
@@ -526,8 +526,7 @@ static int service_search_req(sdp_req_t *req, sdp_buf_t *buf)
}
done:
- if (cstate)
- free(cstate);
+ free(cstate);
if (pattern)
sdp_list_free(pattern, free);
@@ -745,8 +744,7 @@ static int service_attr_req(sdp_req_t *req, sdp_buf_t *buf)
buf->buf_size += sizeof(uint16_t);
done:
- if (cstate)
- free(cstate);
+ free(cstate);
if (seq)
sdp_list_free(seq, free);
if (status)
@@ -929,10 +927,8 @@ static int service_search_attr_req(sdp_req_t *req, sdp_buf_t *buf)
}
done:
- if (cstate)
- free(cstate);
- if (tmpbuf.data)
- free(tmpbuf.data);
+ free(cstate);
+ free(tmpbuf.data);
if (pattern)
sdp_list_free(pattern, free);
if (seq)
diff --git a/src/storage.c b/src/storage.c
index ed1e5ec..07c1ac2 100644
--- a/src/storage.c
+++ b/src/storage.c
@@ -763,8 +763,7 @@ int write_trust(const char *src, const char *addr, const char *service,
/* If the old setting is the same as the requested one, we're done */
if (trusted == trust) {
g_slist_free(services);
- if (str)
- free(str);
+ free(str);
return 0;
}
@@ -784,8 +783,7 @@ int write_trust(const char *src, const char *addr, const char *service,
g_slist_free(services);
- if (str)
- free(str);
+ free(str);
return ret;
}
diff --git a/test/ipctest.c b/test/ipctest.c
index 1a7543b..9fdfac4 100644
--- a/test/ipctest.c
+++ b/test/ipctest.c
@@ -1029,8 +1029,7 @@ static gboolean input_cb(GIOChannel *gin, GIOCondition condition, gpointer data)
if (sscanf(line, "%*s %as", &address) != 1)
DBG("set with bdaddr BDADDR");
- if (u->address)
- free(u->address);
+ free(u->address);
u->address = address;
DBG("bdaddr %s", u->address);
@@ -1049,8 +1048,7 @@ static gboolean input_cb(GIOChannel *gin, GIOCondition condition, gpointer data)
DBG("set with profile [hsp|a2dp]");
}
- if (profile)
- free(profile);
+ free(profile);
DBG("profile %s", u->transport == BT_CAPABILITIES_TRANSPORT_SCO ?
"hsp" : "a2dp");
}
--
1.7.1
^ permalink raw reply related
* [PATCH 1/3] sbc: Fix redundant null check on calling free()
From: Gustavo F. Padovan @ 2010-06-05 10:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
In-Reply-To: <1275732870-15889-1-git-send-email-gustavo@padovan.org>
Issues found by smatch static check: http://smatch.sourceforge.net/
---
sbc/sbc.c | 3 +--
sbc/sbcdec.c | 9 +++------
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/sbc/sbc.c b/sbc/sbc.c
index edd152f..86399dd 100644
--- a/sbc/sbc.c
+++ b/sbc/sbc.c
@@ -1138,8 +1138,7 @@ void sbc_finish(sbc_t *sbc)
if (!sbc)
return;
- if (sbc->priv_alloc_base)
- free(sbc->priv_alloc_base);
+ free(sbc->priv_alloc_base);
memset(sbc, 0, sizeof(sbc_t));
}
diff --git a/sbc/sbcdec.c b/sbc/sbcdec.c
index 77b8aed..7008e88 100644
--- a/sbc/sbcdec.c
+++ b/sbc/sbcdec.c
@@ -259,15 +259,13 @@ int main(int argc, char *argv[])
break;
case 'd':
- if (output)
- free(output);
+ free(output);
output = strdup(optarg);
tofile = 0;
break;
case 'f' :
- if (output)
- free(output);
+ free(output);
output = strdup(optarg);
tofile = 1;
break;
@@ -289,8 +287,7 @@ int main(int argc, char *argv[])
for (i = 0; i < argc; i++)
decode(argv[i], output ? output : "/dev/dsp", tofile);
- if (output)
- free(output);
+ free(output);
return 0;
}
--
1.7.1
^ permalink raw reply related
* Fixes issues reported by Smatch static checker
From: Gustavo F. Padovan @ 2010-06-05 10:14 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo
Issues found by smatch static check: http://smatch.sourceforge.net/
There are still 3 critical issues:
src/storage.c +773 write_trust(29) error: we previously assumed 'match' could be null.
lib/sdp.c +2052 sdp_get_lang_attr(16) warn: variable dereferenced before check 'pEncoding'
lib/sdp.c +2067 sdp_get_lang_attr(31) error: we previously assumed 'pOffset' could be null.
^ permalink raw reply
* [PATCH 14/14] Bluetooth: Fix missing retransmission action with RR(P=1)
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-14-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
The Bluetooth SIG Profile Tuning Suite Software uses the CSA1 spec
to run the L2CAP tests. The new 3.0 spec has a missing
Retransmit-I-Frames action when the Remote side is Busy.
We still start the retransmission timer if Remote is Busy and unacked
frames > 0. We do everything we did before this change plus the
Retransmission of I-frames.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 4a737b2..1c35ff2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3401,8 +3401,8 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
}
- if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
- __mod_retrans_timer();
+ if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY)
+ l2cap_retransmit_frames(sk);
spin_lock_bh(&pi->send_lock);
l2cap_ertm_send(sk);
--
1.7.1
^ permalink raw reply related
* [PATCH 13/14] Bluetooth: Fix SREJ_QUEUE corruption
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, João Paulo Rechi Vita
In-Reply-To: <1275724218-29453-13-git-send-email-gustavo@padovan.org>
From: João Paulo Rechi Vita <jprvita@profusion.mobi>
Since all TxSeq values are modulo, we can't compare them directly. We
have to compare their offset inside the TxWindow instead.
Signed-off-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
Acked-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 59c4690..4a737b2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3418,6 +3418,8 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_seq, u8 sar)
{
struct sk_buff *next_skb;
+ struct l2cap_pinfo *pi = l2cap_pi(sk);
+ int tx_seq_offset, next_tx_seq_offset;
bt_cb(skb)->tx_seq = tx_seq;
bt_cb(skb)->sar = sar;
@@ -3428,11 +3430,20 @@ static int l2cap_add_to_srej_queue(struct sock *sk, struct sk_buff *skb, u8 tx_s
return 0;
}
+ tx_seq_offset = (tx_seq - pi->buffer_seq) % 64;
+ if (tx_seq_offset < 0)
+ tx_seq_offset += 64;
+
do {
if (bt_cb(next_skb)->tx_seq == tx_seq)
return -EINVAL;
- if (bt_cb(next_skb)->tx_seq > tx_seq) {
+ next_tx_seq_offset = (bt_cb(next_skb)->tx_seq -
+ pi->buffer_seq) % 64;
+ if (next_tx_seq_offset < 0)
+ next_tx_seq_offset += 64;
+
+ if (next_tx_seq_offset > tx_seq_offset) {
__skb_queue_before(SREJ_QUEUE(sk), next_skb, skb);
return 0;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 12/14] Bluetooth: Fix l2cap_ertm_send() behavior
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-12-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
This patch makes l2cap_ertm_send() similar to the Send-Data action of
the ERTM spec. We do not have to check for RemoteBusy or WAIT_F state
inside l2cap_ertm_send(). Such check was causing a bug in the
retransmission logic of ERTM.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index f116307..59c4690 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1449,11 +1449,7 @@ static int l2cap_ertm_send(struct sock *sk)
if (sk->sk_state != BT_CONNECTED)
return -ENOTCONN;
- if (pi->conn_state & L2CAP_CONN_WAIT_F)
- return 0;
-
- while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk)) &&
- !(pi->conn_state & L2CAP_CONN_REMOTE_BUSY)) {
+ while ((skb = sk->sk_send_head) && (!l2cap_tx_window_full(sk))) {
if (pi->remote_max_tx &&
bt_cb(skb)->retries == pi->remote_max_tx) {
@@ -1828,6 +1824,11 @@ static int l2cap_sock_sendmsg(struct kiocb *iocb, struct socket *sock, struct ms
if (pi->mode == L2CAP_MODE_STREAMING) {
err = l2cap_streaming_send(sk);
} else {
+ if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY &&
+ pi->conn_state && L2CAP_CONN_WAIT_F) {
+ err = len;
+ break;
+ }
spin_lock_bh(&pi->send_lock);
err = l2cap_ertm_send(sk);
spin_unlock_bh(&pi->send_lock);
@@ -3403,8 +3404,6 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
__mod_retrans_timer();
- pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY;
-
spin_lock_bh(&pi->send_lock);
l2cap_ertm_send(sk);
spin_unlock_bh(&pi->send_lock);
--
1.7.1
^ permalink raw reply related
* [PATCH 11/14] Bluetooth: Fix handle of received P-bit
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-11-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
This is a protocol error. We shall send a F-bit in response
to any P-bit received.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e5b766d..f116307 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1420,8 +1420,15 @@ static void l2cap_retransmit_one_frame(struct sock *sk, u8 tx_seq)
tx_skb = skb_clone(skb, GFP_ATOMIC);
bt_cb(skb)->retries++;
control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
+
+ if (pi->conn_state & L2CAP_CONN_SEND_FBIT) {
+ control |= L2CAP_CTRL_FINAL;
+ pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
+ }
+
control |= (pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT)
| (tx_seq << L2CAP_CTRL_TXSEQ_SHIFT);
+
put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE);
if (pi->fcs == L2CAP_FCS_CRC16) {
@@ -3383,7 +3390,6 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
u16 control = 0;
pi->frames_sent = 0;
- pi->conn_state |= L2CAP_CONN_SEND_FBIT;
control |= pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;
@@ -3952,6 +3958,7 @@ static inline void l2cap_data_channel_rrframe(struct sock *sk, u16 rx_control)
l2cap_drop_acked_frames(sk);
if (rx_control & L2CAP_CTRL_POLL) {
+ pi->conn_state |= L2CAP_CONN_SEND_FBIT;
if (pi->conn_state & L2CAP_CONN_SREJ_SENT) {
if ((pi->conn_state & L2CAP_CONN_REMOTE_BUSY) &&
(pi->unacked_frames > 0))
@@ -4019,6 +4026,8 @@ static inline void l2cap_data_channel_srejframe(struct sock *sk, u16 rx_control)
if (rx_control & L2CAP_CTRL_POLL) {
pi->expected_ack_seq = tx_seq;
l2cap_drop_acked_frames(sk);
+
+ pi->conn_state |= L2CAP_CONN_SEND_FBIT;
l2cap_retransmit_one_frame(sk, tx_seq);
spin_lock_bh(&pi->send_lock);
@@ -4053,6 +4062,9 @@ static inline void l2cap_data_channel_rnrframe(struct sock *sk, u16 rx_control)
pi->expected_ack_seq = tx_seq;
l2cap_drop_acked_frames(sk);
+ if (rx_control & L2CAP_CTRL_POLL)
+ pi->conn_state |= L2CAP_CONN_SEND_FBIT;
+
if (!(pi->conn_state & L2CAP_CONN_SREJ_SENT)) {
del_timer(&pi->retrans_timer);
if (rx_control & L2CAP_CTRL_POLL)
--
1.7.1
^ permalink raw reply related
* [PATCH 10/14] Bluetooth: Fix crash when sending frames after connection is closed
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-10-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
At the time the channel is closed we can't really know if the timer
was really deleted, since we used del_timer(). We can't call
del_timer_sync() in interrupt context! So sometimes the acktimeout
expires and try to send a acknowledgement, but we don't have any
connection anymore.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 3db0078..e5b766d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -345,8 +345,12 @@ static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
struct sk_buff *skb;
struct l2cap_hdr *lh;
struct l2cap_conn *conn = pi->conn;
+ struct sock *sk = (struct sock *)pi;
int count, hlen = L2CAP_HDR_SIZE + 2;
+ if (sk->sk_state != BT_CONNECTED)
+ return;
+
if (pi->fcs == L2CAP_FCS_CRC16)
hlen += 2;
--
1.7.1
^ permalink raw reply related
* [PATCH 09/14] Bluetooth: Fix ERTM channel shutdown
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-9-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
After send a Disconnection Request we shall no send and receive frames
anymore. So we set it to BT_DISCONN when a Disconnection Request is
sent then L2CAP will not be able to send or receive any new packet, as
specified by L2CAP spec.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 50 +++++++++++++++++++++++++++++++++---------------
1 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 35b7791..3db0078 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -428,14 +428,41 @@ static void l2cap_do_start(struct sock *sk)
}
}
+static inline void l2cap_ertm_shutdown(struct sock *sk)
+{
+ struct srej_list *l, *tmp;
+
+ del_timer(&l2cap_pi(sk)->retrans_timer);
+ del_timer(&l2cap_pi(sk)->monitor_timer);
+ del_timer(&l2cap_pi(sk)->ack_timer);
+
+ skb_queue_purge(SREJ_QUEUE(sk));
+ skb_queue_purge(BUSY_QUEUE(sk));
+
+ list_for_each_entry_safe(l, tmp, SREJ_LIST(sk), list) {
+ list_del(&l->list);
+ kfree(l);
+ }
+}
+
static void l2cap_send_disconn_req(struct l2cap_conn *conn, struct sock *sk)
{
struct l2cap_disconn_req req;
+ skb_queue_purge(TX_QUEUE(sk));
+
+ if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
+ del_timer(&l2cap_pi(sk)->retrans_timer);
+ del_timer(&l2cap_pi(sk)->monitor_timer);
+ del_timer(&l2cap_pi(sk)->ack_timer);
+ }
+
req.dcid = cpu_to_le16(l2cap_pi(sk)->dcid);
req.scid = cpu_to_le16(l2cap_pi(sk)->scid);
l2cap_send_cmd(conn, l2cap_get_ident(conn),
L2CAP_DISCONN_REQ, sizeof(req), &req);
+
+ sk->sk_state = BT_DISCONN;
}
/* ---- L2CAP connections ---- */
@@ -726,7 +753,6 @@ static void __l2cap_sock_close(struct sock *sk, int reason)
sk->sk_type == SOCK_STREAM) {
struct l2cap_conn *conn = l2cap_pi(sk)->conn;
- sk->sk_state = BT_DISCONN;
l2cap_sock_set_timer(sk, sk->sk_sndtimeo);
l2cap_send_disconn_req(conn, sk);
} else
@@ -1409,6 +1435,9 @@ static int l2cap_ertm_send(struct sock *sk)
u16 control, fcs;
int nsent = 0;
+ if (sk->sk_state != BT_CONNECTED)
+ return -ENOTCONN;
+
if (pi->conn_state & L2CAP_CONN_WAIT_F)
return 0;
@@ -3068,7 +3097,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr
default:
sk->sk_state = BT_DISCONN;
- sk->sk_err = ECONNRESET;
l2cap_sock_set_timer(sk, HZ * 5);
l2cap_send_disconn_req(conn, sk);
goto done;
@@ -3123,13 +3151,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
skb_queue_purge(TX_QUEUE(sk));
- if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
- skb_queue_purge(SREJ_QUEUE(sk));
- skb_queue_purge(BUSY_QUEUE(sk));
- del_timer(&l2cap_pi(sk)->retrans_timer);
- del_timer(&l2cap_pi(sk)->monitor_timer);
- del_timer(&l2cap_pi(sk)->ack_timer);
- }
+ if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM)
+ l2cap_ertm_shutdown(sk);
l2cap_chan_del(sk, ECONNRESET);
bh_unlock_sock(sk);
@@ -3155,13 +3178,8 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
skb_queue_purge(TX_QUEUE(sk));
- if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM) {
- skb_queue_purge(SREJ_QUEUE(sk));
- skb_queue_purge(BUSY_QUEUE(sk));
- del_timer(&l2cap_pi(sk)->retrans_timer);
- del_timer(&l2cap_pi(sk)->monitor_timer);
- del_timer(&l2cap_pi(sk)->ack_timer);
- }
+ if (l2cap_pi(sk)->mode == L2CAP_MODE_ERTM)
+ l2cap_ertm_shutdown(sk);
l2cap_chan_del(sk, 0);
bh_unlock_sock(sk);
--
1.7.1
^ permalink raw reply related
* [PATCH 08/14] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-8-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
The ack_timer is implemation specific, disabling it in such situation
avoids some potencial errors in the ERTM protocol.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 7a8454a..35b7791 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3632,6 +3632,8 @@ static int l2cap_push_rx_skb(struct sock *sk, struct sk_buff *skb, u16 control)
pi->conn_state |= L2CAP_CONN_RNR_SENT;
+ del_timer(&pi->ack_timer);
+
queue_work(_busy_wq, &pi->busy_work);
return err;
@@ -3881,6 +3883,8 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
pi->conn_state |= L2CAP_CONN_SEND_PBIT;
l2cap_send_srejframe(sk, tx_seq);
+
+ del_timer(&pi->ack_timer);
}
return 0;
--
1.7.1
^ permalink raw reply related
* [PATCH 07/14] Bluetooth: Only check SAR bits if frame is I-frame
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-7-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
Check the SAR bit only make sense for an I-frame. Also check SAR for
S-frame lead to errors.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 97584d8..7a8454a 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4122,7 +4122,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (l2cap_check_fcs(pi, skb))
goto drop;
- if (__is_sar_start(control))
+ if (__is_sar_start(control) && __is_iframe(control))
len -= 2;
if (pi->fcs == L2CAP_FCS_CRC16)
--
1.7.1
^ permalink raw reply related
* [PATCH 06/14] Bluetooth: Check packet FCS earlier
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-6-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
This way, if FCS is enabled and the packet is corrupted, we just drop it
without read it len, which could be corrupted.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
---
net/bluetooth/l2cap.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a567614..97584d8 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4114,25 +4114,25 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
skb_pull(skb, 2);
len = skb->len;
+ /*
+ * We can just drop the corrupted I-frame here.
+ * Receiver will miss it and start proper recovery
+ * procedures and ask retransmission.
+ */
+ if (l2cap_check_fcs(pi, skb))
+ goto drop;
+
if (__is_sar_start(control))
len -= 2;
if (pi->fcs == L2CAP_FCS_CRC16)
len -= 2;
- /*
- * We can just drop the corrupted I-frame here.
- * Receiver will miss it and start proper recovery
- * procedures and ask retransmission.
- */
if (len > pi->mps) {
l2cap_send_disconn_req(pi->conn, sk);
goto drop;
}
- if (l2cap_check_fcs(pi, skb))
- goto drop;
-
req_seq = __get_reqseq(control);
req_seq_offset = (req_seq - pi->expected_ack_seq) % 64;
if (req_seq_offset < 0)
@@ -4172,6 +4172,9 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
skb_pull(skb, 2);
len = skb->len;
+ if (l2cap_check_fcs(pi, skb))
+ goto drop;
+
if (__is_sar_start(control))
len -= 2;
@@ -4181,9 +4184,6 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (len > pi->mps || len < 4 || __is_sframe(control))
goto drop;
- if (l2cap_check_fcs(pi, skb))
- goto drop;
-
tx_seq = __get_txseq(control);
if (pi->expected_tx_seq == tx_seq)
--
1.7.1
^ permalink raw reply related
* [PATCH 05/14] Bluetooth: Fix ERTM vars increment
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-5-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
All ERTM operations regarding the txWindow should be modulo 64,
otherwise we screw up ERTM logic.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 27e69f6..a567614 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3745,7 +3745,7 @@ static void l2cap_check_srej_gap(struct sock *sk, u8 tx_seq)
l2cap_ertm_reassembly_sdu(sk, skb, control);
l2cap_pi(sk)->buffer_seq_srej =
(l2cap_pi(sk)->buffer_seq_srej + 1) % 64;
- tx_seq++;
+ tx_seq = (tx_seq + 1) % 64;
}
}
@@ -3781,10 +3781,11 @@ static void l2cap_send_srejframe(struct sock *sk, u8 tx_seq)
l2cap_send_sframe(pi, control);
new = kzalloc(sizeof(struct srej_list), GFP_ATOMIC);
- new->tx_seq = pi->expected_tx_seq++;
+ new->tx_seq = pi->expected_tx_seq;
+ pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
list_add_tail(&new->list, SREJ_LIST(sk));
}
- pi->expected_tx_seq++;
+ pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
}
static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, struct sk_buff *skb)
--
1.7.1
^ permalink raw reply related
* [PATCH 04/14] Bluetooth: Check skb_clone return to avoid NULL dereference
From: Gustavo F. Padovan @ 2010-06-05 7:50 UTC (permalink / raw)
To: linux-bluetooth; +Cc: gustavo, marcel, Gustavo F. Padovan
In-Reply-To: <1275724218-29453-4-git-send-email-gustavo@padovan.org>
From: Gustavo F. Padovan <padovan@profusion.mobi>
skb can be NULL and a Ooops can happen if we do not check for NULL.
Signed-off-by: Gustavo F. Padovan <padovan@profusion.mobi>
Reviewed-by: João Paulo Rechi Vita <jprvita@profusion.mobi>
---
net/bluetooth/l2cap.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index b08731d..27e69f6 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1335,6 +1335,8 @@ static int l2cap_streaming_send(struct sock *sk)
while ((skb = sk->sk_send_head)) {
tx_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!tx_skb)
+ break;
control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT;
@@ -1420,6 +1422,8 @@ static int l2cap_ertm_send(struct sock *sk)
}
tx_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!tx_skb)
+ break;
bt_cb(skb)->retries++;
--
1.7.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox