* [PATCH 04/10] plugins: use btd_error_failed()
From: Gustavo F. Padovan @ 2010-12-13 21:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292275987-13799-4-git-send-email-padovan@profusion.mobi>
---
plugins/service.c | 22 ++++------------------
1 files changed, 4 insertions(+), 18 deletions(-)
diff --git a/plugins/service.c b/plugins/service.c
index a442d53..f44aa92 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -337,17 +337,6 @@ static void exit_callback(DBusConnection *conn, void *user_data)
g_free(user_record);
}
-static inline DBusMessage *failed(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "Failed");
-}
-
-static inline DBusMessage *failed_strerror(DBusMessage *msg, int err)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "%s", strerror(err));
-}
-
static int add_xml_record(DBusConnection *conn, const char *sender,
struct service_adapter *serv_adapter,
const char *record, dbus_uint32_t *handle)
@@ -412,9 +401,7 @@ static DBusMessage *update_record(DBusConnection *conn, DBusMessage *msg,
if (err < 0) {
sdp_record_free(sdp_record);
error("Failed to update the service record");
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".Failed",
- "%s", strerror(EIO));
+ return btd_error_failed(msg, strerror(-err));
}
return dbus_message_new_method_return(msg);
@@ -449,9 +436,8 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
if (!sdp_record) {
error("Parsing of XML service record failed");
sdp_record_free(sdp_record);
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".Failed",
- "%s", strerror(EIO));
+ return btd_error_failed(msg,
+ "Parsing of XML service record failed");
}
return update_record(conn, msg, serv_adapter, handle, sdp_record);
@@ -494,7 +480,7 @@ static DBusMessage *add_service_record(DBusConnection *conn,
sender = dbus_message_get_sender(msg);
err = add_xml_record(conn, sender, serv_adapter, record, &handle);
if (err < 0)
- return failed_strerror(msg, err);
+ return btd_error_failed(msg, strerror(-err));
reply = dbus_message_new_method_return(msg);
if (!reply)
--
1.7.3.2
^ permalink raw reply related
* [PATCH 03/10] Add btd_error_failed()
From: Gustavo F. Padovan @ 2010-12-13 21:33 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292275987-13799-3-git-send-email-padovan@profusion.mobi>
This is a special error type. It has a more general meaning and allows you
to add a string to the error.
---
src/error.c | 6 ++++++
src/error.h | 1 +
2 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/src/error.c b/src/error.c
index 013de96..3a78628 100644
--- a/src/error.c
+++ b/src/error.c
@@ -114,3 +114,9 @@ DBusMessage *btd_error_no_such_adapter(DBusMessage *msg)
return g_dbus_create_error(msg, ERROR_INTERFACE ".NoSuchAdapter",
"No such adapter");
}
+
+DBusMessage *btd_error_failed(DBusMessage *msg, const char *str)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".Failed", "%s", str);
+}
diff --git a/src/error.h b/src/error.h
index a4e32fe..faaef0a 100644
--- a/src/error.h
+++ b/src/error.h
@@ -41,3 +41,4 @@ DBusMessage *btd_error_in_progress(DBusMessage *msg);
DBusMessage *btd_error_does_not_exist(DBusMessage *msg);
DBusMessage *btd_error_not_authorized(DBusMessage *msg);
DBusMessage *btd_error_no_such_adapter(DBusMessage *msg);
+DBusMessage *btd_error_failed(DBusMessage *msg, const char *str);
--
1.7.3.2
^ permalink raw reply related
* [PATCH 02/10] src: use btd_error_invalid_args()
From: Gustavo F. Padovan @ 2010-12-13 21:32 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292275987-13799-2-git-send-email-padovan@profusion.mobi>
---
src/device.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/device.c b/src/device.c
index 7ceac8b..0f4dc0b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -605,7 +605,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &pattern,
DBUS_TYPE_INVALID) == FALSE)
- goto fail;
+ return btd_error_invalid_args(msg);
if (strlen(pattern) == 0) {
err = device_browse(device, conn, msg, NULL, FALSE);
--
1.7.3.2
^ permalink raw reply related
* [PATCH 01/10] Add btd_error_no_such_adapter()
From: Gustavo F. Padovan @ 2010-12-13 21:32 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292275987-13799-1-git-send-email-padovan@profusion.mobi>
---
src/device.c | 6 ------
src/error.c | 6 ++++++
src/error.h | 1 +
src/manager.c | 13 +++----------
4 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/device.c b/src/device.c
index 91b9d23..7ceac8b 100644
--- a/src/device.c
+++ b/src/device.c
@@ -171,12 +171,6 @@ static DBusHandlerResult error_failed_errno(DBusConnection *conn,
return error_failed(conn, msg, desc);
}
-static inline DBusMessage *no_such_adapter(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NoSuchAdapter",
- "No such adapter");
-}
-
static void browse_request_free(struct browse_req *req)
{
if (req->listener_id)
diff --git a/src/error.c b/src/error.c
index e1f0598..013de96 100644
--- a/src/error.c
+++ b/src/error.c
@@ -108,3 +108,9 @@ DBusMessage *btd_error_not_authorized(DBusMessage *msg)
return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
"Operation Not Authorized");
}
+
+DBusMessage *btd_error_no_such_adapter(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".NoSuchAdapter",
+ "No such adapter");
+}
diff --git a/src/error.h b/src/error.h
index 9d80fa0..a4e32fe 100644
--- a/src/error.h
+++ b/src/error.h
@@ -40,3 +40,4 @@ DBusMessage *btd_error_not_available(DBusMessage *msg);
DBusMessage *btd_error_in_progress(DBusMessage *msg);
DBusMessage *btd_error_does_not_exist(DBusMessage *msg);
DBusMessage *btd_error_not_authorized(DBusMessage *msg);
+DBusMessage *btd_error_no_such_adapter(DBusMessage *msg);
diff --git a/src/manager.c b/src/manager.c
index ccaa1a2..c8ec7e5 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -59,13 +59,6 @@ const char *manager_get_base_path(void)
return base_path;
}
-static inline DBusMessage *no_such_adapter(DBusMessage *msg)
-{
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NoSuchAdapter",
- "No such adapter");
-}
-
static DBusMessage *default_adapter(DBusConnection *conn,
DBusMessage *msg, void *data)
{
@@ -75,7 +68,7 @@ static DBusMessage *default_adapter(DBusConnection *conn,
adapter = manager_find_adapter_by_id(default_adapter_id);
if (!adapter)
- return no_such_adapter(msg);
+ return btd_error_no_such_adapter(msg);
reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -108,7 +101,7 @@ static DBusMessage *find_adapter(DBusConnection *conn,
path = adapter_any_get_path();
if (path != NULL)
goto done;
- return no_such_adapter(msg);
+ return btd_error_no_such_adapter(msg);
} else if (!strncmp(pattern, "hci", 3) && strlen(pattern) >= 4) {
dev_id = atoi(pattern + 3);
adapter = manager_find_adapter_by_id(dev_id);
@@ -116,7 +109,7 @@ static DBusMessage *find_adapter(DBusConnection *conn,
adapter = manager_find_adapter_by_address(pattern);
if (!adapter)
- return no_such_adapter(msg);
+ return btd_error_no_such_adapter(msg);
path = adapter_get_path(adapter);
--
1.7.3.2
^ permalink raw reply related
* [PATCH 00/10] More btd_error_* patches
From: Gustavo F. Padovan @ 2010-12-13 21:32 UTC (permalink / raw)
To: linux-bluetooth
Hi,
This patch set adds a new error function btd_error_failed(), which has a str
parameter to enable a more accurate error report in the case the usual
btd_error_* does not fit.
After this patch set there will 10 occurrences of g_dbus_create_error() inside
bluez code (not counting health/ here)
Please review!
Gustavo F. Padovan (10):
Add btd_error_no_such_adapter()
src: use btd_error_invalid_args()
Add btd_error_failed()
plugins: use btd_error_failed()
attrib: use btd_error_failed()
network: use btd_error_failed()
input: use btd_error_failed()
audio: use btd_error_failed()
serial: use btd_error_failed()
src: use btd_error_failed()
attrib/client.c | 10 +-------
audio/control.c | 6 +---
audio/device.c | 14 ++++--------
audio/gateway.c | 11 ++++-----
audio/headset.c | 18 ++++------------
audio/sink.c | 9 ++-----
audio/source.c | 9 ++-----
audio/transport.c | 7 +-----
input/device.c | 31 +++++++----------------------
network/connection.c | 30 +--------------------------
network/server.c | 13 ++---------
plugins/service.c | 22 +++-----------------
serial/port.c | 20 ++++++++----------
serial/proxy.c | 8 +-----
src/adapter.c | 37 ++++++++++++-----------------------
src/device.c | 52 ++++++++++++-------------------------------------
src/error.c | 12 +++++++++++
src/error.h | 2 +
src/manager.c | 13 ++---------
19 files changed, 97 insertions(+), 227 deletions(-)
--
1.7.3.2
^ permalink raw reply
* Re: [PATCH 1/1] Support for reading long Characteristic Values.
From: Vinicius Costa Gomes @ 2010-12-13 20:33 UTC (permalink / raw)
To: Brian Gix; +Cc: linux-bluetooth, padovan, rshaffer
In-Reply-To: <4D067A6C.6080404@codeaurora.org>
On 11:56 Mon 13 Dec, Brian Gix wrote:
> Hi Vinicius,
>
> On 12/13/2010 10:39 AM, Vinicius Costa Gomes wrote:
> > On 09:24 Mon 13 Dec, Brian Gix wrote:
> >> Modify existing gatt_read_char() function support the reading of
> >> attributes (specifically Characteristic Values) that are longer than
> >> the MTU would otherwise allow. When a result to an ATT_OP_READ_REQ
> >> is received, it will be passed to the requester as always if the total
> >> result was shorter than the Default MTU (23). Any results equal to or
> >> longer will cause a series of READ_BLOB requests to be made, with the
> >> additional results built up until the end of the Attribute is detected.
> >> The full result will then be passed to the original requester.
> >>
> >> The end of the Attribute is detected by either a successful result to
> >> the READ_BLOB request that is shorter than the Default MTU, or by an
> >> error result that indicates that a read is being attempted beyond the
> >> length of the attribute, or that the Attribute wasn't "Long".
> >>
> >> This patch is dependant on the earlier patch:
> >> 0001-Implempent-READ_BLOB-encoding-for-ATT.patch
> >>
> >> The packet composed conforms to the Bluetooth Core v4.0 specification.
> >>
> >> Brian Gix
> >> bgix@codeaurora.org
> >> Employee of Qualcomm Innovation Center, Inc.
> >> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> I will omit the sig in the annotation in the future.
>
> Also, I will examine the subsequent submissions for non-TAB ws. I need
> to work on my gvim settings a bit, I think.
>
> >>
> >> ---
> >> attrib/gatt.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >> 1 files changed, 112 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/attrib/gatt.c b/attrib/gatt.c
> >> index bca8b49..d888f1d 100644
> >> --- a/attrib/gatt.c
> >> +++ b/attrib/gatt.c
> >> @@ -97,15 +97,125 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
> >> pdu, plen, func, user_data, NULL);
> >> }
> >>
> >> +struct read_long_data {
> >> + GAttrib *attrib;
> >> + GAttribResultFunc func;
> >> + gpointer user_data;
> >> + guint8 *result_data;
> >> + guint16 result_len;
> >
> > I would call result_data "buffer" (or something like it) and result_len "size"
> > (or total). What do you think?
>
> Will rename to buffer and size. I often avoid "size" to avoid keyword
> collisions, but I don't believe it applies here.
>
> >
> >> + guint16 handle;
> >> +};
> >> +
> >> +static void read_blob_helper(guint8 status, const guint8 *res_pdu,
> >> + guint16 res_len, gpointer user_data)
> >> +{
> >> + struct read_long_data *long_read = user_data;
> >> + uint8_t pdu[ATT_DEFAULT_MTU];
> >> + guint8 *tmp;
> >> + guint16 plen;
> >> + guint ret_val;
> >> +
> >> + if (status == ATT_ECODE_ATTR_NOT_LONG ||
> >> + status == ATT_ECODE_INVALID_OFFSET) {
> >> + status = 0;
> >> + goto done;
> >> + }
> >> +
> >> + if (status != 0 || res_len == 1)
> >> + goto done;
> >> +
> >> + tmp = g_try_realloc(long_read->result_data,
> >> + long_read->result_len + res_len - 1);
> >> +
> >> + if (tmp == NULL) {
> >> + status = ATT_ECODE_INSUFF_RESOURCES;
> >> + goto done;
> >> + }
> >> +
> >> + memcpy(&tmp[long_read->result_len],&res_pdu[1], res_len-1);
> >
> > It should be "res - 1".
> >
> >> + long_read->result_data = tmp;
> >> + long_read->result_len += res_len-1;
> >
> > Same here.
> >
> >> +
> >> + if (res_len< ATT_DEFAULT_MTU)
> >> + goto done;
> >> +
> >> + plen = enc_read_blob_req(long_read->handle, long_read->result_len-1,
> >
> > And here.
>
> done, done, done.
>
> >
> >> + pdu, sizeof(pdu));
> >> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ, pdu,
> >> + plen, read_blob_helper, long_read, NULL);
> >> +
> >> + if (ret_val != 0)
> >> + return;
> >> +
> >> + status = ATT_ECODE_INSUFF_RESOURCES;
> >> +
> >> +done:
> >> + long_read->func(status, long_read->result_data, long_read->result_len,
> >> + long_read->user_data);
> >> + g_free(long_read->result_data);
> >> + g_free(long_read);
> >> +}
> >> +
> >> +static void read_char_helper(guint8 status, const guint8 *res_pdu,
> >> + guint16 res_len, gpointer user_data)
> >> +{
> >> + struct read_long_data *long_read = user_data;
> >> + uint8_t pdu[ATT_DEFAULT_MTU];
> >> + guint16 plen;
> >> + guint ret_val;
> >> +
> >> + if (status != 0 || res_len< ATT_DEFAULT_MTU)
> >> + goto done;
> >> +
> >> + long_read->result_data = g_malloc(res_len);
> >> +
> >> + if (long_read->result_data == NULL)
> >> + goto done;
> >> +
> >> + memcpy(long_read->result_data, res_pdu, res_len);
> >> + long_read->result_len = res_len;
> >> +
> >> + plen = enc_read_blob_req(long_read->handle, res_len-1, pdu,
> >> + sizeof(pdu));
> >> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ,
> >> + pdu, plen, read_blob_helper, long_read, NULL);
> >
> > g_attrib_send() returns an command id (something that could be used to cancel
> > the command later, if needed), so I think it would make more sense to call
> > ret_val "id" or just "ret". And thinking about it, I guess that the "res_"
> > prefix doesn't add much meaning to "res_pdu" and "res_len". Do you agree?
>
> renamed res_pdu to "rpdu", to avoid name collision with outbound local
> pdu variable.
>
> renamed res_len to "rlen" to keep the two together, and distinct from
> plen, which is also used for the outbound pdu.
>
> renamed ret_val to "ret". In two of the functions, it has lost the
> ability to be used to cancel the GATT procedure. It can cancel the
> original ATT_OP_READ_REQ opcodes, but not the subsequent
> ATT_OP_READ_BLOB_REQ opcodes.
>
> >
> >> +
> >> + if (ret_val != 0)
> >> + return;
> >> +
> >> + status = ATT_ECODE_INSUFF_RESOURCES;
> >> +
> >> +done:
> >> + long_read->func(status, res_pdu, res_len, long_read->user_data);
> >
> > If g_attrib_send() fails, load_read->result_data may be leaking here.
>
> Is there no guarantee that the GAttribResultFunc parameter will be
> invoked if there is a non-Zero return value from g_attrib_send?
>
> If there is paths that could result in a non-response (abnormal link
> termination?) then you are correct, and I will need to rethink clean-up.
The case I was referring was when g_attrib_send() cannot allocate memory for
the command.
>
> If I am correctly reading the code, I can pass a function to the
> GDestroyNotify parameter of g_attrib_send, which will alert me to the
> destruction of the command, at which point I could do clean-up. Is that
> path foolproof? This would also entail removing all g_free clean-ups
> from the helper functions, because the destroy function is always
> called, including after invoking the GAttribResultFunc function.
>
Yeah, this path should be foolproof. The destroy function should be called
even for abnormal cases, the ResultFunc is called just when the command
finishes (with a response or an error).
> Also:
> If we intend to use the ID of the (original) command to cancel the GATT
> procedure, I propose the following:
>
> Compound GATT procedure such as the Read Long procedure that I am
> implementing, should save the original ID returned by g_attrib_send in
> the nested user_data structure such as I have done. When subsequent ATT
> commands are sent as part of the same GATT procedure, the new command
> shall be assigned the same ID as the original. This could be done with
> a function in gattrib.c with the prototype:
>
> g_attrib_set_id(GAttrib *attrib, guint old_id, guint new_id);
>
> This would allow the upper layer to cancel the entire GATT procedure,
> even if it is partially completed.
>
> This same methodology could then be applied to long writes, and other
> compound GATT procedures.
>
> What do you think?
Sounds good, I just don't know about messing with the commands id's,
maybe having some thing like command groups.
For example, adding these funtions:
guint g_attrib_send_full(GAttrib *attrib, guint8 opcode, const guint8 *pdu,
guint16 len, guint group, GAttribResultFunc func,
gpointer user_data, GDestroyNotify notify);
gboolean g_attrib_cancel_group(GAttrib *attrib, guint group);
How do this look?
>
> >
> >> + g_free(long_read);
> >> +}
> >> +
> >> guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
> >> gpointer user_data)
> >> {
> >> uint8_t pdu[ATT_DEFAULT_MTU];
> >> guint16 plen;
> >> + guint ret_val;
> >> + struct read_long_data *long_read;
> >> +
> >> + long_read = g_try_new0(struct read_long_data, 1);
> >> +
> >> + if (long_read == NULL)
> >> + return 0;
> >> +
> >> + long_read->attrib = attrib;
> >> + long_read->func = func;
> >> + long_read->user_data = user_data;
> >> + long_read->handle = handle;
> >>
> >> plen = enc_read_req(handle, pdu, sizeof(pdu));
> >> - return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
> >> - user_data, NULL);
> >> + ret_val = g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen,
> >> + read_char_helper, long_read, NULL);
> >> +
> >> + if (ret_val == 0)
> >> + g_free(long_read);
> >> +
> >> + return ret_val;
> >> }
> >>
> >> guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
> >> --
> >> 1.7.1
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> >> the body of a message to majordomo@vger.kernel.org
> >> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > Cheers,
>
> Thanks,
>
> --
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 1/1] Support for reading long Characteristic Values.
From: Brian Gix @ 2010-12-13 19:56 UTC (permalink / raw)
To: Vinicius Costa Gomes; +Cc: linux-bluetooth, padovan, rshaffer
In-Reply-To: <20101213183951.GB7549@eris>
Hi Vinicius,
On 12/13/2010 10:39 AM, Vinicius Costa Gomes wrote:
> On 09:24 Mon 13 Dec, Brian Gix wrote:
>> Modify existing gatt_read_char() function support the reading of
>> attributes (specifically Characteristic Values) that are longer than
>> the MTU would otherwise allow. When a result to an ATT_OP_READ_REQ
>> is received, it will be passed to the requester as always if the total
>> result was shorter than the Default MTU (23). Any results equal to or
>> longer will cause a series of READ_BLOB requests to be made, with the
>> additional results built up until the end of the Attribute is detected.
>> The full result will then be passed to the original requester.
>>
>> The end of the Attribute is detected by either a successful result to
>> the READ_BLOB request that is shorter than the Default MTU, or by an
>> error result that indicates that a read is being attempted beyond the
>> length of the attribute, or that the Attribute wasn't "Long".
>>
>> This patch is dependant on the earlier patch:
>> 0001-Implempent-READ_BLOB-encoding-for-ATT.patch
>>
>> The packet composed conforms to the Bluetooth Core v4.0 specification.
>>
>> Brian Gix
>> bgix@codeaurora.org
>> Employee of Qualcomm Innovation Center, Inc.
>> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
I will omit the sig in the annotation in the future.
Also, I will examine the subsequent submissions for non-TAB ws. I need
to work on my gvim settings a bit, I think.
>>
>> ---
>> attrib/gatt.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 files changed, 112 insertions(+), 2 deletions(-)
>>
>> diff --git a/attrib/gatt.c b/attrib/gatt.c
>> index bca8b49..d888f1d 100644
>> --- a/attrib/gatt.c
>> +++ b/attrib/gatt.c
>> @@ -97,15 +97,125 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
>> pdu, plen, func, user_data, NULL);
>> }
>>
>> +struct read_long_data {
>> + GAttrib *attrib;
>> + GAttribResultFunc func;
>> + gpointer user_data;
>> + guint8 *result_data;
>> + guint16 result_len;
>
> I would call result_data "buffer" (or something like it) and result_len "size"
> (or total). What do you think?
Will rename to buffer and size. I often avoid "size" to avoid keyword
collisions, but I don't believe it applies here.
>
>> + guint16 handle;
>> +};
>> +
>> +static void read_blob_helper(guint8 status, const guint8 *res_pdu,
>> + guint16 res_len, gpointer user_data)
>> +{
>> + struct read_long_data *long_read = user_data;
>> + uint8_t pdu[ATT_DEFAULT_MTU];
>> + guint8 *tmp;
>> + guint16 plen;
>> + guint ret_val;
>> +
>> + if (status == ATT_ECODE_ATTR_NOT_LONG ||
>> + status == ATT_ECODE_INVALID_OFFSET) {
>> + status = 0;
>> + goto done;
>> + }
>> +
>> + if (status != 0 || res_len == 1)
>> + goto done;
>> +
>> + tmp = g_try_realloc(long_read->result_data,
>> + long_read->result_len + res_len - 1);
>> +
>> + if (tmp == NULL) {
>> + status = ATT_ECODE_INSUFF_RESOURCES;
>> + goto done;
>> + }
>> +
>> + memcpy(&tmp[long_read->result_len],&res_pdu[1], res_len-1);
>
> It should be "res - 1".
>
>> + long_read->result_data = tmp;
>> + long_read->result_len += res_len-1;
>
> Same here.
>
>> +
>> + if (res_len< ATT_DEFAULT_MTU)
>> + goto done;
>> +
>> + plen = enc_read_blob_req(long_read->handle, long_read->result_len-1,
>
> And here.
done, done, done.
>
>> + pdu, sizeof(pdu));
>> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ, pdu,
>> + plen, read_blob_helper, long_read, NULL);
>> +
>> + if (ret_val != 0)
>> + return;
>> +
>> + status = ATT_ECODE_INSUFF_RESOURCES;
>> +
>> +done:
>> + long_read->func(status, long_read->result_data, long_read->result_len,
>> + long_read->user_data);
>> + g_free(long_read->result_data);
>> + g_free(long_read);
>> +}
>> +
>> +static void read_char_helper(guint8 status, const guint8 *res_pdu,
>> + guint16 res_len, gpointer user_data)
>> +{
>> + struct read_long_data *long_read = user_data;
>> + uint8_t pdu[ATT_DEFAULT_MTU];
>> + guint16 plen;
>> + guint ret_val;
>> +
>> + if (status != 0 || res_len< ATT_DEFAULT_MTU)
>> + goto done;
>> +
>> + long_read->result_data = g_malloc(res_len);
>> +
>> + if (long_read->result_data == NULL)
>> + goto done;
>> +
>> + memcpy(long_read->result_data, res_pdu, res_len);
>> + long_read->result_len = res_len;
>> +
>> + plen = enc_read_blob_req(long_read->handle, res_len-1, pdu,
>> + sizeof(pdu));
>> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ,
>> + pdu, plen, read_blob_helper, long_read, NULL);
>
> g_attrib_send() returns an command id (something that could be used to cancel
> the command later, if needed), so I think it would make more sense to call
> ret_val "id" or just "ret". And thinking about it, I guess that the "res_"
> prefix doesn't add much meaning to "res_pdu" and "res_len". Do you agree?
renamed res_pdu to "rpdu", to avoid name collision with outbound local
pdu variable.
renamed res_len to "rlen" to keep the two together, and distinct from
plen, which is also used for the outbound pdu.
renamed ret_val to "ret". In two of the functions, it has lost the
ability to be used to cancel the GATT procedure. It can cancel the
original ATT_OP_READ_REQ opcodes, but not the subsequent
ATT_OP_READ_BLOB_REQ opcodes.
>
>> +
>> + if (ret_val != 0)
>> + return;
>> +
>> + status = ATT_ECODE_INSUFF_RESOURCES;
>> +
>> +done:
>> + long_read->func(status, res_pdu, res_len, long_read->user_data);
>
> If g_attrib_send() fails, load_read->result_data may be leaking here.
Is there no guarantee that the GAttribResultFunc parameter will be
invoked if there is a non-Zero return value from g_attrib_send?
If there is paths that could result in a non-response (abnormal link
termination?) then you are correct, and I will need to rethink clean-up.
If I am correctly reading the code, I can pass a function to the
GDestroyNotify parameter of g_attrib_send, which will alert me to the
destruction of the command, at which point I could do clean-up. Is that
path foolproof? This would also entail removing all g_free clean-ups
from the helper functions, because the destroy function is always
called, including after invoking the GAttribResultFunc function.
Also:
If we intend to use the ID of the (original) command to cancel the GATT
procedure, I propose the following:
Compound GATT procedure such as the Read Long procedure that I am
implementing, should save the original ID returned by g_attrib_send in
the nested user_data structure such as I have done. When subsequent ATT
commands are sent as part of the same GATT procedure, the new command
shall be assigned the same ID as the original. This could be done with
a function in gattrib.c with the prototype:
g_attrib_set_id(GAttrib *attrib, guint old_id, guint new_id);
This would allow the upper layer to cancel the entire GATT procedure,
even if it is partially completed.
This same methodology could then be applied to long writes, and other
compound GATT procedures.
What do you think?
>
>> + g_free(long_read);
>> +}
>> +
>> guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
>> gpointer user_data)
>> {
>> uint8_t pdu[ATT_DEFAULT_MTU];
>> guint16 plen;
>> + guint ret_val;
>> + struct read_long_data *long_read;
>> +
>> + long_read = g_try_new0(struct read_long_data, 1);
>> +
>> + if (long_read == NULL)
>> + return 0;
>> +
>> + long_read->attrib = attrib;
>> + long_read->func = func;
>> + long_read->user_data = user_data;
>> + long_read->handle = handle;
>>
>> plen = enc_read_req(handle, pdu, sizeof(pdu));
>> - return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
>> - user_data, NULL);
>> + ret_val = g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen,
>> + read_char_helper, long_read, NULL);
>> +
>> + if (ret_val == 0)
>> + g_free(long_read);
>> +
>> + return ret_val;
>> }
>>
>> guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Cheers,
Thanks,
--
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [PATCH 5/5] Bluetooth: Add management events for controller addition & removal
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292267227-22028-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch adds Bluetooth Management interface events for controller
addition and removal. The events correspond to the existing HCI_DEV_REG
and HCI_DEV_UNREG stack internal events.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/hci_core.h | 2 +
include/net/bluetooth/mgmt.h | 10 +++++++++
net/bluetooth/hci_core.c | 2 +
net/bluetooth/mgmt.c | 41 ++++++++++++++++++++++++++++++++++++++
4 files changed, 55 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1992fac..3786ee8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -662,6 +662,8 @@ void hci_send_to_sock(struct hci_dev *hdev, struct sk_buff *skb);
/* Management interface */
int mgmt_control(struct sock *sk, struct msghdr *msg, size_t len);
+int mgmt_index_added(u16 index);
+int mgmt_index_removed(u16 index);
/* HCI info for socket */
#define hci_pi(sk) ((struct hci_pinfo *) sk)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 70985aa..ca29c13 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -75,3 +75,13 @@ struct mgmt_ev_controller_error {
__le16 index;
__u8 error_code;
} __packed;
+
+#define MGMT_EV_INDEX_ADDED 0x0004
+struct mgmt_ev_index_added {
+ __le16 index;
+} __packed;
+
+#define MGMT_EV_INDEX_REMOVED 0x0005
+struct mgmt_ev_index_removed {
+ __le16 index;
+} __packed;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 51c61f7..1a4ec97 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -960,6 +960,7 @@ int hci_register_dev(struct hci_dev *hdev)
}
}
+ mgmt_index_added(hdev->id);
hci_notify(hdev, HCI_DEV_REG);
return id;
@@ -989,6 +990,7 @@ int hci_unregister_dev(struct hci_dev *hdev)
for (i = 0; i < NUM_REASSEMBLY; i++)
kfree_skb(hdev->reassembly[i]);
+ mgmt_index_removed(hdev->id);
hci_notify(hdev, HCI_DEV_UNREG);
if (hdev->rfkill) {
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d6c5a32..f827fd9 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -265,3 +265,44 @@ done:
kfree(buf);
return err;
}
+
+static int mgmt_event(u16 event, void *data, u16 data_len)
+{
+ struct sk_buff *skb;
+ struct mgmt_hdr *hdr;
+
+ skb = alloc_skb(sizeof(*hdr) + data_len, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ bt_cb(skb)->channel = HCI_CHANNEL_CONTROL;
+
+ hdr = (void *) skb_put(skb, sizeof(*hdr));
+ hdr->opcode = cpu_to_le16(event);
+ hdr->len = cpu_to_le16(data_len);
+
+ memcpy(skb_put(skb, data_len), data, data_len);
+
+ hci_send_to_sock(NULL, skb);
+ kfree_skb(skb);
+
+ return 0;
+}
+
+int mgmt_index_added(u16 index)
+{
+ struct mgmt_ev_index_added ev;
+
+ put_unaligned_le16(index, &ev.index);
+
+ return mgmt_event(MGMT_EV_INDEX_ADDED, &ev, sizeof(ev));
+}
+
+int mgmt_index_removed(u16 index)
+{
+ struct mgmt_ev_index_added ev;
+
+ put_unaligned_le16(index, &ev.index);
+
+ return mgmt_event(MGMT_EV_INDEX_REMOVED, &ev, sizeof(ev));
+}
--
1.7.2.3
^ permalink raw reply related
* [PATCH 4/5] Bluetooth: Add read_info management command
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292267227-22028-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch implements the read_info command which is used to fetch basic
info about an adapter.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/mgmt.h | 19 +++++++++
net/bluetooth/mgmt.c | 90 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 101 insertions(+), 8 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index c2b4c83..70985aa 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -39,6 +39,25 @@ struct mgmt_rp_read_index_list {
__le16 index[0];
} __packed;
+#define MGMT_OP_READ_INFO 0x0004
+struct mgmt_cp_read_info {
+ __le16 index;
+} __packed;
+struct mgmt_rp_read_info {
+ __le16 index;
+ __u8 type;
+ __u8 powered;
+ __u8 discoverable;
+ __u8 pairable;
+ __u8 sec_mode;
+ bdaddr_t bdaddr;
+ __u8 dev_class[3];
+ __u8 features[8];
+ __u16 manufacturer;
+ __u8 hci_ver;
+ __u16 hci_rev;
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7a8e321..d6c5a32 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -32,6 +32,33 @@
#define MGMT_VERSION 0
#define MGMT_REVISION 1
+static int cmd_status(struct sock *sk, u16 cmd, u8 status)
+{
+ struct sk_buff *skb;
+ struct mgmt_hdr *hdr;
+ struct mgmt_ev_cmd_status *ev;
+
+ BT_DBG("sock %p", sk);
+
+ skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = (void *) skb_put(skb, sizeof(*hdr));
+
+ hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
+ hdr->len = cpu_to_le16(sizeof(*ev));
+
+ ev = (void *) skb_put(skb, sizeof(*ev));
+ ev->status = status;
+ put_unaligned_le16(cmd, &ev->opcode);
+
+ if (sock_queue_rcv_skb(sk, skb) < 0)
+ kfree_skb(skb);
+
+ return 0;
+}
+
static int read_version(struct sock *sk)
{
struct sk_buff *skb;
@@ -112,26 +139,70 @@ static int read_index_list(struct sock *sk)
return 0;
}
-static int cmd_status(struct sock *sk, u16 cmd, u8 status)
+static int read_controller_info(struct sock *sk, unsigned char *data, u16 len)
{
struct sk_buff *skb;
struct mgmt_hdr *hdr;
- struct mgmt_ev_cmd_status *ev;
+ struct mgmt_ev_cmd_complete *ev;
+ struct mgmt_rp_read_info *rp;
+ struct mgmt_cp_read_info *cp;
+ struct hci_dev *hdev;
+ u16 dev_id;
BT_DBG("sock %p", sk);
- skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
+ if (len != 2)
+ return cmd_status(sk, MGMT_OP_READ_INFO, EINVAL);
+
+ skb = alloc_skb(sizeof(*hdr) + sizeof(*ev) + sizeof(*rp), GFP_ATOMIC);
if (!skb)
return -ENOMEM;
hdr = (void *) skb_put(skb, sizeof(*hdr));
-
- hdr->opcode = cpu_to_le16(MGMT_EV_CMD_STATUS);
- hdr->len = cpu_to_le16(sizeof(*ev));
+ hdr->opcode = cpu_to_le16(MGMT_EV_CMD_COMPLETE);
+ hdr->len = cpu_to_le16(sizeof(*ev) + sizeof(*rp));
ev = (void *) skb_put(skb, sizeof(*ev));
- ev->status = status;
- put_unaligned_le16(cmd, &ev->opcode);
+ put_unaligned_le16(MGMT_OP_READ_INFO, &ev->opcode);
+
+ rp = (void *) skb_put(skb, sizeof(*rp));
+
+ cp = (void *) data;
+ dev_id = get_unaligned_le16(&cp->index);
+
+ BT_DBG("request for hci%u", dev_id);
+
+ hdev = hci_dev_get(dev_id);
+ if (!hdev) {
+ kfree_skb(skb);
+ return cmd_status(sk, MGMT_OP_READ_INFO, ENODEV);
+ }
+
+ hci_dev_lock_bh(hdev);
+
+ put_unaligned_le16(hdev->id, &rp->index);
+ rp->type = hdev->dev_type;
+
+ rp->powered = test_bit(HCI_UP, &hdev->flags);
+ rp->discoverable = test_bit(HCI_ISCAN, &hdev->flags);
+ rp->pairable = test_bit(HCI_PSCAN, &hdev->flags);
+
+ if (test_bit(HCI_AUTH, &hdev->flags))
+ rp->sec_mode = 3;
+ else if (hdev->ssp_mode > 0)
+ rp->sec_mode = 4;
+ else
+ rp->sec_mode = 2;
+
+ bacpy(&rp->bdaddr, &hdev->bdaddr);
+ memcpy(rp->features, hdev->features, 8);
+ memcpy(rp->dev_class, hdev->dev_class, 3);
+ put_unaligned_le16(hdev->manufacturer, &rp->manufacturer);
+ rp->hci_ver = hdev->hci_ver;
+ put_unaligned_le16(hdev->hci_rev, &rp->hci_rev);
+
+ hci_dev_unlock_bh(hdev);
+ hci_dev_put(hdev);
if (sock_queue_rcv_skb(sk, skb) < 0)
kfree_skb(skb);
@@ -176,6 +247,9 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_READ_INDEX_LIST:
err = read_index_list(sk);
break;
+ case MGMT_OP_READ_INFO:
+ err = read_controller_info(sk, buf + sizeof(*hdr), len);
+ break;
default:
BT_DBG("Unknown op %u", opcode);
err = cmd_status(sk, opcode, 0x01);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 3/5] Bluetooth: Add read_index_list management command
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292267227-22028-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch implements the read_index_list command through which
userspace can get a list of current adapter indices.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/mgmt.h | 6 ++++
net/bluetooth/mgmt.c | 53 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index d353d64..c2b4c83 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -33,6 +33,12 @@ struct mgmt_rp_read_version {
__le16 revision;
} __packed;
+#define MGMT_OP_READ_INDEX_LIST 0x0003
+struct mgmt_rp_read_index_list {
+ __le16 num_controllers;
+ __le16 index[0];
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3e24c0b..7a8e321 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -62,6 +62,56 @@ static int read_version(struct sock *sk)
return 0;
}
+static int read_index_list(struct sock *sk)
+{
+ struct sk_buff *skb;
+ struct mgmt_hdr *hdr;
+ struct mgmt_ev_cmd_complete *ev;
+ struct mgmt_rp_read_index_list *rp;
+ struct list_head *p;
+ size_t body_len;
+ u16 count;
+ int i;
+
+ BT_DBG("sock %p", sk);
+
+ read_lock(&hci_dev_list_lock);
+
+ count = 0;
+ list_for_each(p, &hci_dev_list) {
+ count++;
+ }
+
+ body_len = sizeof(*ev) + sizeof(*rp) + (2 * count);
+ skb = alloc_skb(sizeof(*hdr) + body_len, GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = (void *) skb_put(skb, sizeof(*hdr));
+ hdr->opcode = cpu_to_le16(MGMT_EV_CMD_COMPLETE);
+ hdr->len = cpu_to_le16(body_len);
+
+ ev = (void *) skb_put(skb, sizeof(*ev));
+ put_unaligned_le16(MGMT_OP_READ_INDEX_LIST, &ev->opcode);
+
+ rp = (void *) skb_put(skb, sizeof(*rp) + (2 * count));
+ put_unaligned_le16(count, &rp->num_controllers);
+
+ i = 0;
+ list_for_each(p, &hci_dev_list) {
+ struct hci_dev *d = list_entry(p, struct hci_dev, list);
+ put_unaligned_le16(d->id, &rp->index[i++]);
+ BT_DBG("Added hci%u", d->id);
+ }
+
+ read_unlock(&hci_dev_list_lock);
+
+ if (sock_queue_rcv_skb(sk, skb) < 0)
+ kfree_skb(skb);
+
+ return 0;
+}
+
static int cmd_status(struct sock *sk, u16 cmd, u8 status)
{
struct sk_buff *skb;
@@ -123,6 +173,9 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
case MGMT_OP_READ_VERSION:
err = read_version(sk);
break;
+ case MGMT_OP_READ_INDEX_LIST:
+ err = read_index_list(sk);
+ break;
default:
BT_DBG("Unknown op %u", opcode);
err = cmd_status(sk, opcode, 0x01);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 2/5] Bluetooth: Add read_version management command
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292267227-22028-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch implements the initial read_version command that userspace
will use before any other management interface operations.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/mgmt.h | 6 ++++++
net/bluetooth/mgmt.c | 36 ++++++++++++++++++++++++++++++++++++
2 files changed, 42 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/mgmt.h b/include/net/bluetooth/mgmt.h
index 95974da..d353d64 100644
--- a/include/net/bluetooth/mgmt.h
+++ b/include/net/bluetooth/mgmt.h
@@ -27,6 +27,12 @@ struct mgmt_hdr {
} __packed;
#define MGMT_HDR_SIZE 4
+#define MGMT_OP_READ_VERSION 0x0001
+struct mgmt_rp_read_version {
+ __u8 version;
+ __le16 revision;
+} __packed;
+
#define MGMT_EV_CMD_COMPLETE 0x0001
struct mgmt_ev_cmd_complete {
__le16 opcode;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7ea5489..3e24c0b 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -29,6 +29,39 @@
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/mgmt.h>
+#define MGMT_VERSION 0
+#define MGMT_REVISION 1
+
+static int read_version(struct sock *sk)
+{
+ struct sk_buff *skb;
+ struct mgmt_hdr *hdr;
+ struct mgmt_ev_cmd_complete *ev;
+ struct mgmt_rp_read_version *rp;
+
+ BT_DBG("sock %p", sk);
+
+ skb = alloc_skb(sizeof(*hdr) + sizeof(*ev) + sizeof(*rp), GFP_ATOMIC);
+ if (!skb)
+ return -ENOMEM;
+
+ hdr = (void *) skb_put(skb, sizeof(*hdr));
+ hdr->opcode = cpu_to_le16(MGMT_EV_CMD_COMPLETE);
+ hdr->len = cpu_to_le16(sizeof(*ev) + sizeof(*rp));
+
+ ev = (void *) skb_put(skb, sizeof(*ev));
+ put_unaligned_le16(MGMT_OP_READ_VERSION, &ev->opcode);
+
+ rp = (void *) skb_put(skb, sizeof(*rp));
+ rp->version = MGMT_VERSION;
+ put_unaligned_le16(MGMT_REVISION, &rp->revision);
+
+ if (sock_queue_rcv_skb(sk, skb) < 0)
+ kfree_skb(skb);
+
+ return 0;
+}
+
static int cmd_status(struct sock *sk, u16 cmd, u8 status)
{
struct sk_buff *skb;
@@ -87,6 +120,9 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
}
switch (opcode) {
+ case MGMT_OP_READ_VERSION:
+ err = read_version(sk);
+ break;
default:
BT_DBG("Unknown op %u", opcode);
err = cmd_status(sk, opcode, 0x01);
--
1.7.2.3
^ permalink raw reply related
* [PATCH 1/5] Bluetooth: Add error handling for managment command handlers
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292267227-22028-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@nokia.com>
The command handlers for bluetooth management messaging should be able
to report errors (such as memory allocation failures) to the higher
levels in the call stack.
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
net/bluetooth/mgmt.c | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index d15bf67..7ea5489 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -29,7 +29,7 @@
#include <net/bluetooth/hci_core.h>
#include <net/bluetooth/mgmt.h>
-static void cmd_status(struct sock *sk, u16 cmd, u8 status)
+static int cmd_status(struct sock *sk, u16 cmd, u8 status)
{
struct sk_buff *skb;
struct mgmt_hdr *hdr;
@@ -39,7 +39,7 @@ static void cmd_status(struct sock *sk, u16 cmd, u8 status)
skb = alloc_skb(sizeof(*hdr) + sizeof(*ev), GFP_ATOMIC);
if (!skb)
- return;
+ return -ENOMEM;
hdr = (void *) skb_put(skb, sizeof(*hdr));
@@ -52,6 +52,8 @@ static void cmd_status(struct sock *sk, u16 cmd, u8 status)
if (sock_queue_rcv_skb(sk, skb) < 0)
kfree_skb(skb);
+
+ return 0;
}
int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
@@ -87,10 +89,13 @@ int mgmt_control(struct sock *sk, struct msghdr *msg, size_t msglen)
switch (opcode) {
default:
BT_DBG("Unknown op %u", opcode);
- cmd_status(sk, opcode, 0x01);
+ err = cmd_status(sk, opcode, 0x01);
break;
}
+ if (err < 0)
+ goto done;
+
err = msglen;
done:
--
1.7.2.3
^ permalink raw reply related
* Basic Management interface command handling
From: johan.hedberg @ 2010-12-13 19:07 UTC (permalink / raw)
To: linux-bluetooth
Hi,
Here's a set of patches for dealing with basic commands needed for local
adapter enumeration and information featching.
I'm not completely sure about the parameter list of read_info (maybe
something is missing or too much) but we can fine tune that later as
long as the interface is experimental. The basic idea is to avoid too
many messages to get the initial set of info about an adapter and then
have separate "value X changed" events from the kernel to userspace when
individual values change. For the values that can be changed by the user
there'll also be individual "set value" commands.
With this set of patches you should be able to get the enumeration (at
least somewhat) working with current bluez.git:
echo 1 > /sys/module/bluetooth/parameters/enable_mgmt
bluetoothd -n -d -p mgmtops
There's no power control (HCI_UP/HCI_DOWN) available yet, so you'll get
more meaningful values in read_info if you do "hciconfig hci0 up" before
running bluetoothd.
Johan
^ permalink raw reply
* Re: [PATCH 1/1] Support for reading long Characteristic Values.
From: Vinicius Costa Gomes @ 2010-12-13 18:39 UTC (permalink / raw)
To: Brian Gix; +Cc: linux-bluetooth, padovan, rshaffer
In-Reply-To: <1292261066-28100-3-git-send-email-bgix@codeaurora.org>
On 09:24 Mon 13 Dec, Brian Gix wrote:
> Modify existing gatt_read_char() function support the reading of
> attributes (specifically Characteristic Values) that are longer than
> the MTU would otherwise allow. When a result to an ATT_OP_READ_REQ
> is received, it will be passed to the requester as always if the total
> result was shorter than the Default MTU (23). Any results equal to or
> longer will cause a series of READ_BLOB requests to be made, with the
> additional results built up until the end of the Attribute is detected.
> The full result will then be passed to the original requester.
>
> The end of the Attribute is detected by either a successful result to
> the READ_BLOB request that is shorter than the Default MTU, or by an
> error result that indicates that a read is being attempted beyond the
> length of the attribute, or that the Attribute wasn't "Long".
>
> This patch is dependant on the earlier patch:
> 0001-Implempent-READ_BLOB-encoding-for-ATT.patch
>
> The packet composed conforms to the Bluetooth Core v4.0 specification.
>
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
>
> ---
> attrib/gatt.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 112 insertions(+), 2 deletions(-)
>
> diff --git a/attrib/gatt.c b/attrib/gatt.c
> index bca8b49..d888f1d 100644
> --- a/attrib/gatt.c
> +++ b/attrib/gatt.c
> @@ -97,15 +97,125 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
> pdu, plen, func, user_data, NULL);
> }
>
> +struct read_long_data {
> + GAttrib *attrib;
> + GAttribResultFunc func;
> + gpointer user_data;
> + guint8 *result_data;
> + guint16 result_len;
I would call result_data "buffer" (or something like it) and result_len "size"
(or total). What do you think?
> + guint16 handle;
> +};
> +
> +static void read_blob_helper(guint8 status, const guint8 *res_pdu,
> + guint16 res_len, gpointer user_data)
> +{
> + struct read_long_data *long_read = user_data;
> + uint8_t pdu[ATT_DEFAULT_MTU];
> + guint8 *tmp;
> + guint16 plen;
> + guint ret_val;
> +
> + if (status == ATT_ECODE_ATTR_NOT_LONG ||
> + status == ATT_ECODE_INVALID_OFFSET) {
> + status = 0;
> + goto done;
> + }
> +
> + if (status != 0 || res_len == 1)
> + goto done;
> +
> + tmp = g_try_realloc(long_read->result_data,
> + long_read->result_len + res_len - 1);
> +
> + if (tmp == NULL) {
> + status = ATT_ECODE_INSUFF_RESOURCES;
> + goto done;
> + }
> +
> + memcpy(&tmp[long_read->result_len], &res_pdu[1], res_len-1);
It should be "res - 1".
> + long_read->result_data = tmp;
> + long_read->result_len += res_len-1;
Same here.
> +
> + if (res_len < ATT_DEFAULT_MTU)
> + goto done;
> +
> + plen = enc_read_blob_req(long_read->handle, long_read->result_len-1,
And here.
> + pdu, sizeof(pdu));
> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ, pdu,
> + plen, read_blob_helper, long_read, NULL);
> +
> + if (ret_val != 0)
> + return;
> +
> + status = ATT_ECODE_INSUFF_RESOURCES;
> +
> +done:
> + long_read->func(status, long_read->result_data, long_read->result_len,
> + long_read->user_data);
> + g_free(long_read->result_data);
> + g_free(long_read);
> +}
> +
> +static void read_char_helper(guint8 status, const guint8 *res_pdu,
> + guint16 res_len, gpointer user_data)
> +{
> + struct read_long_data *long_read = user_data;
> + uint8_t pdu[ATT_DEFAULT_MTU];
> + guint16 plen;
> + guint ret_val;
> +
> + if (status != 0 || res_len < ATT_DEFAULT_MTU)
> + goto done;
> +
> + long_read->result_data = g_malloc(res_len);
> +
> + if (long_read->result_data == NULL)
> + goto done;
> +
> + memcpy(long_read->result_data, res_pdu, res_len);
> + long_read->result_len = res_len;
> +
> + plen = enc_read_blob_req(long_read->handle, res_len-1, pdu,
> + sizeof(pdu));
> + ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ,
> + pdu, plen, read_blob_helper, long_read, NULL);
g_attrib_send() returns an command id (something that could be used to cancel
the command later, if needed), so I think it would make more sense to call
ret_val "id" or just "ret". And thinking about it, I guess that the "res_"
prefix doesn't add much meaning to "res_pdu" and "res_len". Do you agree?
> +
> + if (ret_val != 0)
> + return;
> +
> + status = ATT_ECODE_INSUFF_RESOURCES;
> +
> +done:
> + long_read->func(status, res_pdu, res_len, long_read->user_data);
If g_attrib_send() fails, load_read->result_data may be leaking here.
> + g_free(long_read);
> +}
> +
> guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
> gpointer user_data)
> {
> uint8_t pdu[ATT_DEFAULT_MTU];
> guint16 plen;
> + guint ret_val;
> + struct read_long_data *long_read;
> +
> + long_read = g_try_new0(struct read_long_data, 1);
> +
> + if (long_read == NULL)
> + return 0;
> +
> + long_read->attrib = attrib;
> + long_read->func = func;
> + long_read->user_data = user_data;
> + long_read->handle = handle;
>
> plen = enc_read_req(handle, pdu, sizeof(pdu));
> - return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
> - user_data, NULL);
> + ret_val = g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen,
> + read_char_helper, long_read, NULL);
> +
> + if (ret_val == 0)
> + g_free(long_read);
> +
> + return ret_val;
> }
>
> guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Vinicius
^ permalink raw reply
* Re: [PATCH 1/1] Implempent READ_BLOB encoding for ATT.
From: Vinicius Costa Gomes @ 2010-12-13 18:15 UTC (permalink / raw)
To: Brian Gix; +Cc: linux-bluetooth, padovan, rshaffer
In-Reply-To: <1292261066-28100-2-git-send-email-bgix@codeaurora.org>
Hi Brian,
On 09:24 Mon 13 Dec, Brian Gix wrote:
> Added enc_read_blob_req() API to ATT transport, to enable
> the reading of attributes that exceed the length of the
> MTU.
>
> The packet composed conforms to the Bluetooth Core v4.0 specification.
>
> Brian Gix
> bgix@codeaurora.org
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
Not sure if this signature belongs in the patch comment.
>
> ---
> attrib/att.c | 18 ++++++++++++++++++
> attrib/att.h | 2 ++
> 2 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/attrib/att.c b/attrib/att.c
> index 445b192..d0879f8 100644
> --- a/attrib/att.c
> +++ b/attrib/att.c
> @@ -542,6 +542,24 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
> return min_len;
> }
>
> +uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
> + int len)
I couldn't apply the patches because there were a mix between tabs and spaces,
please use only tabs for indentation.
> +{
> + const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) + sizeof(offset);
> +
> + if (pdu == NULL)
> + return 0;
> +
> + if (len < min_len)
> + return 0;
> +
> + pdu[0] = ATT_OP_READ_BLOB_REQ;
> + att_put_u16(handle, &pdu[1]);
> + att_put_u16(offset, &pdu[3]);
> +
> + return min_len;
> +}
> +
> uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
> {
> const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
> diff --git a/attrib/att.h b/attrib/att.h
> index 2c8c724..46bdad4 100644
> --- a/attrib/att.h
> +++ b/attrib/att.h
> @@ -202,6 +202,8 @@ uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
> uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
> uint8_t *value, int *vlen);
> uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
> +uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
> + int len);
> uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
> uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
> uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Vinicius
^ permalink raw reply
* Re: [PATCH] btusb: Fix log spamming due to autosuspend
From: Gustavo F. Padovan @ 2010-12-13 17:46 UTC (permalink / raw)
To: Stefan Seyfried
Cc: linux-bluetooth, Marcel Holtmann, gregkh, Stefan Seyfried,
Oliver Neukum
In-Reply-To: <20101209201657.0b7964fc@susi>
Hi Stefan,
* Stefan Seyfried <stefan.seyfried@googlemail.com> [2010-12-09 20:16:57 +0100]:
> Hi all,
>
> On Wed, 1 Dec 2010 15:49:10 -0200
> "Gustavo F. Padovan" <padovan@profusion.mobi> wrote:
>
> > Hi Stefan,
> >
> > * Stefan Seyfried <stefan.seyfried@googlemail.com> [2010-12-01 15:47:14 +0100]:
>
> > > > > If a device is autosuspended an inability to resubmit URBs is
> > > > > to be expected. Check the error code and only log real errors.
> > > > > (Now that autosuspend is default enabled for btusb, those log
> > > > > messages were happening all the time e.g. with a BT mouse)
> > > > >
> > > > > Signed-off-by: Stefan Seyfried <seife+kernel@b1-systems.com>
> > > > > Signed-off-by: Oliver Neukum <oneukum@suse.de>
> > > >
> > > > we had a similar one some time ago, but I am fine with this one as well.
> > > > Actually this one might be a bit better since it still keeps some
> > > > errors.
> > > >
> > > > Acked-by: Marcel Holtmann <marcel@holtmann.org>
> > >
> > > Could you (or Gustavo) send it to Linus? It's pretty trivial, but the
> > > messages are annoying and users will complain if they are still in 2.6.37
> > > final.
> >
> > I'll send it, applied to bluetooth-2.6 tree. Thanks.
>
> unfortunately, it does not seem to make it into 2.6.37?
Patch is in net-2.6 right now. Should go to mainline soon.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* pull request: bluetooth-2.6 2010-12-13
From: Gustavo F. Padovan @ 2010-12-13 17:43 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, linux-bluetooth
Hi John,
The latest Bluetooth patches to linux 2.6.37. First is very critical fix a
break in the DUN profile connection setup. Second patch is also critical and
adds a NULL pointer protection in HCI code.
Please pull or let me know any problems! Thanks.
The following changes since commit 0a54917c3fc295cb61f3fb52373c173fd3b69f48:
orinoco: fix TKIP countermeasure behaviour (2010-12-08 15:24:06 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/padovan/bluetooth-2.6.git master
Johan Hedberg (1):
Bluetooth: Fix initial RFCOMM DLC security level
Jun Nie (1):
Bluetooth: add NULL pointer check in HCI
drivers/bluetooth/hci_ldisc.c | 6 ++++--
net/bluetooth/rfcomm/core.c | 1 +
2 files changed, 5 insertions(+), 2 deletions(-)
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: Start dropping PCMCIA from compat-wireless for 2.6.38+
From: Luis R. Rodriguez @ 2010-12-13 17:26 UTC (permalink / raw)
To: Holger Schurig; +Cc: linux-wireless, linux-kernel, linux-bluetooth
In-Reply-To: <201012130846.16522.holgerschurig@gmail.com>
On Sun, Dec 12, 2010 at 11:46 PM, Holger Schurig
<holgerschurig@gmail.com> wrote:
> Please, if the burden is not too much, keep it for the benefit of
> embedded people.
>
> If you have some embedded device (e.g. ARM based), it's usually not so
> easy to hieve this device to newest 2.6.3[5-7] kernel. However, it's
> easy to use your adapted kernel, and compile compat-wireless against it.
>
> I'm in such a situation with libertas_cs.
You use libertas_cs on an embedded device with compat-wireless, OK,
thanks for the feedback. If you can help with patches when PCMCIA crap
is advanced that would help too.
Luis
^ permalink raw reply
* [PATCH 1/1] Support for reading long Characteristic Values.
From: Brian Gix @ 2010-12-13 17:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, rshaffer, Brian Gix
In-Reply-To: <1292261066-28100-1-git-send-email-bgix@codeaurora.org>
Modify existing gatt_read_char() function support the reading of
attributes (specifically Characteristic Values) that are longer than
the MTU would otherwise allow. When a result to an ATT_OP_READ_REQ
is received, it will be passed to the requester as always if the total
result was shorter than the Default MTU (23). Any results equal to or
longer will cause a series of READ_BLOB requests to be made, with the
additional results built up until the end of the Attribute is detected.
The full result will then be passed to the original requester.
The end of the Attribute is detected by either a successful result to
the READ_BLOB request that is shorter than the Default MTU, or by an
error result that indicates that a read is being attempted beyond the
length of the attribute, or that the Attribute wasn't "Long".
This patch is dependant on the earlier patch:
0001-Implempent-READ_BLOB-encoding-for-ATT.patch
The packet composed conforms to the Bluetooth Core v4.0 specification.
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
---
attrib/gatt.c | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/attrib/gatt.c b/attrib/gatt.c
index bca8b49..d888f1d 100644
--- a/attrib/gatt.c
+++ b/attrib/gatt.c
@@ -97,15 +97,125 @@ guint gatt_read_char_by_uuid(GAttrib *attrib, uint16_t start, uint16_t end,
pdu, plen, func, user_data, NULL);
}
+struct read_long_data {
+ GAttrib *attrib;
+ GAttribResultFunc func;
+ gpointer user_data;
+ guint8 *result_data;
+ guint16 result_len;
+ guint16 handle;
+};
+
+static void read_blob_helper(guint8 status, const guint8 *res_pdu,
+ guint16 res_len, gpointer user_data)
+{
+ struct read_long_data *long_read = user_data;
+ uint8_t pdu[ATT_DEFAULT_MTU];
+ guint8 *tmp;
+ guint16 plen;
+ guint ret_val;
+
+ if (status == ATT_ECODE_ATTR_NOT_LONG ||
+ status == ATT_ECODE_INVALID_OFFSET) {
+ status = 0;
+ goto done;
+ }
+
+ if (status != 0 || res_len == 1)
+ goto done;
+
+ tmp = g_try_realloc(long_read->result_data,
+ long_read->result_len + res_len - 1);
+
+ if (tmp == NULL) {
+ status = ATT_ECODE_INSUFF_RESOURCES;
+ goto done;
+ }
+
+ memcpy(&tmp[long_read->result_len], &res_pdu[1], res_len-1);
+ long_read->result_data = tmp;
+ long_read->result_len += res_len-1;
+
+ if (res_len < ATT_DEFAULT_MTU)
+ goto done;
+
+ plen = enc_read_blob_req(long_read->handle, long_read->result_len-1,
+ pdu, sizeof(pdu));
+ ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ, pdu,
+ plen, read_blob_helper, long_read, NULL);
+
+ if (ret_val != 0)
+ return;
+
+ status = ATT_ECODE_INSUFF_RESOURCES;
+
+done:
+ long_read->func(status, long_read->result_data, long_read->result_len,
+ long_read->user_data);
+ g_free(long_read->result_data);
+ g_free(long_read);
+}
+
+static void read_char_helper(guint8 status, const guint8 *res_pdu,
+ guint16 res_len, gpointer user_data)
+{
+ struct read_long_data *long_read = user_data;
+ uint8_t pdu[ATT_DEFAULT_MTU];
+ guint16 plen;
+ guint ret_val;
+
+ if (status != 0 || res_len < ATT_DEFAULT_MTU)
+ goto done;
+
+ long_read->result_data = g_malloc(res_len);
+
+ if (long_read->result_data == NULL)
+ goto done;
+
+ memcpy(long_read->result_data, res_pdu, res_len);
+ long_read->result_len = res_len;
+
+ plen = enc_read_blob_req(long_read->handle, res_len-1, pdu,
+ sizeof(pdu));
+ ret_val = g_attrib_send(long_read->attrib, ATT_OP_READ_BLOB_REQ,
+ pdu, plen, read_blob_helper, long_read, NULL);
+
+ if (ret_val != 0)
+ return;
+
+ status = ATT_ECODE_INSUFF_RESOURCES;
+
+done:
+ long_read->func(status, res_pdu, res_len, long_read->user_data);
+ g_free(long_read);
+}
+
guint gatt_read_char(GAttrib *attrib, uint16_t handle, GAttribResultFunc func,
gpointer user_data)
{
uint8_t pdu[ATT_DEFAULT_MTU];
guint16 plen;
+ guint ret_val;
+ struct read_long_data *long_read;
+
+ long_read = g_try_new0(struct read_long_data, 1);
+
+ if (long_read == NULL)
+ return 0;
+
+ long_read->attrib = attrib;
+ long_read->func = func;
+ long_read->user_data = user_data;
+ long_read->handle = handle;
plen = enc_read_req(handle, pdu, sizeof(pdu));
- return g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen, func,
- user_data, NULL);
+ ret_val = g_attrib_send(attrib, ATT_OP_READ_REQ, pdu, plen,
+ read_char_helper, long_read, NULL);
+
+ if (ret_val == 0)
+ g_free(long_read);
+
+ return ret_val;
}
guint gatt_write_char(GAttrib *attrib, uint16_t handle, uint8_t *value,
--
1.7.1
^ permalink raw reply related
* [PATCH 1/1] Implempent READ_BLOB encoding for ATT.
From: Brian Gix @ 2010-12-13 17:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, rshaffer, Brian Gix
In-Reply-To: <1292261066-28100-1-git-send-email-bgix@codeaurora.org>
Added enc_read_blob_req() API to ATT transport, to enable
the reading of attributes that exceed the length of the
MTU.
The packet composed conforms to the Bluetooth Core v4.0 specification.
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
---
attrib/att.c | 18 ++++++++++++++++++
attrib/att.h | 2 ++
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index 445b192..d0879f8 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -542,6 +542,24 @@ uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len)
return min_len;
}
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len)
+{
+ const uint16_t min_len = sizeof(pdu[0]) + sizeof(handle) + sizeof(offset);
+
+ if (pdu == NULL)
+ return 0;
+
+ if (len < min_len)
+ return 0;
+
+ pdu[0] = ATT_OP_READ_BLOB_REQ;
+ att_put_u16(handle, &pdu[1]);
+ att_put_u16(offset, &pdu[3]);
+
+ return min_len;
+}
+
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle)
{
const uint16_t min_len = sizeof(pdu[0]) + sizeof(*handle);
diff --git a/attrib/att.h b/attrib/att.h
index 2c8c724..46bdad4 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -202,6 +202,8 @@ uint16_t enc_write_req(uint16_t handle, const uint8_t *value, int vlen,
uint16_t dec_write_req(const uint8_t *pdu, int len, uint16_t *handle,
uint8_t *value, int *vlen);
uint16_t enc_read_req(uint16_t handle, uint8_t *pdu, int len);
+uint16_t enc_read_blob_req(uint16_t handle, uint16_t offset, uint8_t *pdu,
+ int len);
uint16_t dec_read_req(const uint8_t *pdu, int len, uint16_t *handle);
uint16_t enc_read_resp(uint8_t *value, int vlen, uint8_t *pdu, int len);
uint16_t dec_read_resp(const uint8_t *pdu, int len, uint8_t *value, int *vlen);
--
1.7.1
^ permalink raw reply related
* [PATCH 0/2] Implement Long reads for GATT/ATT
From: Brian Gix @ 2010-12-13 17:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: padovan, rshaffer
These patches implement long attribute reads for GATT clients.
The first patch implements the ATT_OP_READ_BLOB_REQ opcode pkt in ATT.
The second patch modifies the gatt_read_char() API to recognize long
attributes, and build up the result over multiple ATT calls to both
ATT_OP_READ_REQ and ATT_OP_READ_BLOB_REQ, until the entire attribute
has been retrieved. It then forwards to the original requester.
The public GATT API is unchanged, and the input parameters, and returned
results are the same.
Over-the-air, the request-result sequence is unchanged unless a long
attribute is detected.
Brian Gix
bgix@codeaurora.org
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum
^ permalink raw reply
* [RFC] Bluetooth: Use non-flushable pb flag by default for ACL data on capable chipsets.
From: Emeltchenko Andrei @ 2010-12-13 14:38 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1292251105-31130-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Modification of Nick Pelly <npelly@google.com> patch.
With Bluetooth 2.1 ACL packets can be flushable or non-flushable. This commit
makes ACL data packets non-flushable by default on compatible chipsets, and
adds the BT_FLUSHABLE socket option to explicitly request flushable ACL
data packets for a given L2CAP socket. This is useful for A2DP data which can
be safely discarded if it can not be delivered within a short time (while
other ACL data should not be discarded).
Note that making ACL data flushable has no effect unless the automatic flush
timeout for that ACL link is changed from its default of 0 (infinite).
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
include/net/bluetooth/bluetooth.h | 5 +++++
include/net/bluetooth/hci.h | 2 ++
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/hci_core.c | 6 ++++--
net/bluetooth/l2cap.c | 33 +++++++++++++++++++++++++++++++--
6 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 0c5e725..ed7d775 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -64,6 +64,11 @@ struct bt_security {
#define BT_DEFER_SETUP 7
+#define BT_FLUSHABLE 8
+
+#define BT_FLUSHABLE_OFF 0
+#define BT_FLUSHABLE_ON 1
+
#define BT_INFO(fmt, arg...) printk(KERN_INFO "Bluetooth: " fmt "\n" , ## arg)
#define BT_ERR(fmt, arg...) printk(KERN_ERR "%s: " fmt "\n" , __func__ , ## arg)
#define BT_DBG(fmt, arg...) pr_debug("%s: " fmt "\n" , __func__ , ## arg)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 29a7a8c..333d5cb 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -150,6 +150,7 @@ enum {
#define EDR_ESCO_MASK (ESCO_2EV3 | ESCO_3EV3 | ESCO_2EV5 | ESCO_3EV5)
/* ACL flags */
+#define ACL_START_NO_FLUSH 0x00
#define ACL_CONT 0x01
#define ACL_START 0x02
#define ACL_ACTIVE_BCAST 0x04
@@ -193,6 +194,7 @@ enum {
#define LMP_EDR_ESCO_3M 0x40
#define LMP_EDR_3S_ESCO 0x80
+#define LMP_NO_FLUSH 0x01
#define LMP_SIMPLE_PAIR 0x08
/* Connection modes */
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 1992fac..9778bc8 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -456,6 +456,7 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
#define lmp_sniffsubr_capable(dev) ((dev)->features[5] & LMP_SNIFF_SUBR)
#define lmp_esco_capable(dev) ((dev)->features[3] & LMP_ESCO)
#define lmp_ssp_capable(dev) ((dev)->features[6] & LMP_SIMPLE_PAIR)
+#define lmp_no_flush_capable(dev) ((dev)->features[6] & LMP_NO_FLUSH)
/* ----- HCI protocols ----- */
struct hci_proto {
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 7ad25ca..af35711 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -75,6 +75,7 @@ struct l2cap_conninfo {
#define L2CAP_LM_TRUSTED 0x0008
#define L2CAP_LM_RELIABLE 0x0010
#define L2CAP_LM_SECURE 0x0020
+#define L2CAP_LM_FLUSHABLE 0x0040
/* L2CAP command codes */
#define L2CAP_COMMAND_REJ 0x01
@@ -327,6 +328,7 @@ struct l2cap_pinfo {
__u8 sec_level;
__u8 role_switch;
__u8 force_reliable;
+ __u8 flushable;
__u8 conf_req[64];
__u8 conf_len;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 51c61f7..c0d776b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1380,7 +1380,7 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_START);
+ hci_add_acl_hdr(skb, conn->handle, flags);
list = skb_shinfo(skb)->frag_list;
if (!list) {
@@ -1398,12 +1398,14 @@ void hci_send_acl(struct hci_conn *conn, struct sk_buff *skb, __u16 flags)
spin_lock_bh(&conn->data_q.lock);
__skb_queue_tail(&conn->data_q, skb);
+ flags &= ~ACL_START;
+ flags |= ACL_CONT;
do {
skb = list; list = list->next;
skb->dev = (void *) hdev;
bt_cb(skb)->pkt_type = HCI_ACLDATA_PKT;
- hci_add_acl_hdr(skb, conn->handle, flags | ACL_CONT);
+ hci_add_acl_hdr(skb, conn->handle, flags);
BT_DBG("%s frag %p len %d", hdev->name, skb, skb->len);
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index c791fcd..c7f25c2 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -362,13 +362,19 @@ static inline u8 l2cap_get_ident(struct l2cap_conn *conn)
static inline void l2cap_send_cmd(struct l2cap_conn *conn, u8 ident, u8 code, u16 len, void *data)
{
struct sk_buff *skb = l2cap_build_cmd(conn, code, ident, len, data);
+ u8 flags;
BT_DBG("code 0x%2.2x", code);
if (!skb)
return;
- hci_send_acl(conn->hcon, skb, 0);
+ if (lmp_no_flush_capable(conn->hcon->hdev))
+ flags = ACL_START_NO_FLUSH;
+ else
+ flags = ACL_START;
+
+ hci_send_acl(conn->hcon, skb, flags);
}
static inline void l2cap_send_sframe(struct l2cap_pinfo *pi, u16 control)
@@ -900,6 +906,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = l2cap_pi(parent)->sec_level;
pi->role_switch = l2cap_pi(parent)->role_switch;
pi->force_reliable = l2cap_pi(parent)->force_reliable;
+ pi->flushable = l2cap_pi(parent)->flushable;
} else {
pi->imtu = L2CAP_DEFAULT_MTU;
pi->omtu = 0;
@@ -915,6 +922,7 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->sec_level = BT_SECURITY_LOW;
pi->role_switch = 0;
pi->force_reliable = 0;
+ pi->flushable = BT_FLUSHABLE_OFF;
}
/* Default config options */
@@ -2098,6 +2106,20 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname, ch
bt_sk(sk)->defer_setup = opt;
break;
+ case BT_FLUSHABLE:
+ if (get_user(opt, (u32 __user *) optval)) {
+ err = -EFAULT;
+ break;
+ }
+
+ if (opt > BT_FLUSHABLE_ON) {
+ err = -EINVAL;
+ break;
+ }
+
+ l2cap_pi(sk)->flushable = opt;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -2237,6 +2259,13 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname, ch
break;
+ case BT_FLUSHABLE:
+
+ if (put_user(l2cap_pi(sk)->flushable, (u32 __user *) optval))
+ err = -EFAULT;
+
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -4697,7 +4726,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon, struct sk_buff *skb, u16 fl
BT_DBG("conn %p len %d flags 0x%x", conn, skb->len, flags);
- if (flags & ACL_START) {
+ if (!(flags & ACL_CONT)) {
struct l2cap_hdr *hdr;
struct sock *sk;
u16 cid;
--
1.7.1
^ permalink raw reply related
* [RFC] Allow Bluez to select flushable or non-flushable ACL packets
From: Emeltchenko Andrei @ 2010-12-13 14:38 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Slight modification of original patch from Nick Pelly <npelly@google.com>
Changes: rebasing, simplifying logic, using SOL_BLUETOOTH instead of SOL_L2CAP.
No changes done for RFCOMM sockets since no use case exist so far but can be
added later
Andrei Emeltchenko (1):
Bluetooth: Use non-flushable pb flag by default for ACL data on
capable chipsets.
include/net/bluetooth/bluetooth.h | 5 +++++
include/net/bluetooth/hci.h | 2 ++
include/net/bluetooth/hci_core.h | 1 +
include/net/bluetooth/l2cap.h | 2 ++
net/bluetooth/hci_core.c | 6 ++++--
net/bluetooth/l2cap.c | 33 +++++++++++++++++++++++++++++++--
6 files changed, 45 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH] Setting default Link Policy according to the chip supported features
From: Johan Hedberg @ 2010-12-13 14:27 UTC (permalink / raw)
To: Pawel Wieczorkiewicz; +Cc: linux-bluetooth
In-Reply-To: <1292249954-8544-1-git-send-email-pawel.wieczorkiewicz@tieto.com>
Hi Pawel,
On Mon, Dec 13, 2010, Pawel Wieczorkiewicz wrote:
> By default all features are enabled (RSWITCH, HOLD, PARK, SNIFF).
> When "read local supported features" complete event occurs, not supported
> features are disabled and then "Write default link policy" command with
> supported features is sent.
>
> On behalf of ST-Ericsson SA
> ---
> plugins/hciops.c | 35 +++++++++++++++--------------------
> 1 files changed, 15 insertions(+), 20 deletions(-)
Thanks! The patch has been pushed upstream.
Johan
^ permalink raw reply
* [PATCH] Setting default Link Policy according to the chip supported features
From: Pawel Wieczorkiewicz @ 2010-12-13 14:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pawel Wieczorkiewicz
By default all features are enabled (RSWITCH, HOLD, PARK, SNIFF).
When "read local supported features" complete event occurs, not supported
features are disabled and then "Write default link policy" command with
supported features is sent.
On behalf of ST-Ericsson SA
---
plugins/hciops.c | 35 +++++++++++++++--------------------
1 files changed, 15 insertions(+), 20 deletions(-)
diff --git a/plugins/hciops.c b/plugins/hciops.c
index 4af137c..44039ba 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -297,6 +297,7 @@ static void start_adapter(int index)
{
uint8_t events[8] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0x1f, 0x00, 0x00 };
uint8_t inqmode;
+ uint16_t policy;
if (VER(index).lmp_ver > 1) {
if (FEATURES(index)[5] & LMP_SNIFF_SUBR)
@@ -345,6 +346,20 @@ static void start_adapter(int index)
hci_send_cmd(SK(index), OGF_HOST_CTL,
OCF_READ_INQ_RESPONSE_TX_POWER_LEVEL, 0, NULL);
+ /* Set default link policy */
+ if (!(FEATURES(index)[0] & LMP_RSWITCH))
+ main_opts.link_policy &= ~HCI_LP_RSWITCH;
+ if (!(FEATURES(index)[0] & LMP_HOLD))
+ main_opts.link_policy &= ~HCI_LP_HOLD;
+ if (!(FEATURES(index)[0] & LMP_SNIFF))
+ main_opts.link_policy &= ~HCI_LP_SNIFF;
+ if (!(FEATURES(index)[1] & LMP_PARK))
+ main_opts.link_policy &= ~HCI_LP_PARK;
+
+ policy = htobs(main_opts.link_policy);
+ hci_send_cmd(SK(index), OGF_LINK_POLICY,
+ OCF_WRITE_DEFAULT_LINK_POLICY, 2, &policy);
+
CURRENT_COD(index) = 0;
memset(EIR(index), 0, sizeof(EIR(index)));
@@ -1949,7 +1964,6 @@ static void at_child_exit(void)
static void device_devup_setup(int index)
{
struct hci_dev_info di;
- uint16_t policy;
read_stored_link_key_cp cp;
DBG("hci%d", index);
@@ -1972,11 +1986,6 @@ static void device_devup_setup(int index)
WRITE_PAGE_TIMEOUT_CP_SIZE, &cp);
}
- /* Set default link policy */
- policy = htobs(main_opts.link_policy);
- hci_send_cmd(SK(index), OGF_LINK_POLICY,
- OCF_WRITE_DEFAULT_LINK_POLICY, 2, &policy);
-
bacpy(&cp.bdaddr, BDADDR_ANY);
cp.read_all = 1;
hci_send_cmd(SK(index), OGF_HOST_CTL, OCF_READ_STORED_LINK_KEY,
@@ -1997,7 +2006,6 @@ static void init_pending(int index)
static void init_device(int index)
{
struct hci_dev_req dr;
- struct hci_dev_info di;
int dd;
pid_t pid;
@@ -2042,19 +2050,6 @@ static void init_device(int index)
error("Can't set link mode on hci%d: %s (%d)",
index, strerror(errno), errno);
- /* Set link policy for BR/EDR HCI devices */
- if (hci_devinfo(index, &di) < 0)
- goto fail;
-
- if (!ignore_device(&di)) {
- dr.dev_opt = main_opts.link_policy;
- if (ioctl(dd, HCISETLINKPOL, (unsigned long) &dr) < 0 &&
- errno != ENETDOWN) {
- error("Can't set link policy on hci%d: %s (%d)",
- index, strerror(errno), errno);
- }
- }
-
/* Start HCI device */
if (ioctl(dd, HCIDEVUP, index) < 0 && errno != EALREADY) {
error("Can't init device hci%d: %s (%d)",
--
1.7.0.4
^ 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