* Re: [PATCH] IrMC sync server support
From: Marcel Holtmann @ 2010-08-12 11:41 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201008121013.o7CADR8r004125@joshua.mesa.nl>
Hi Marcel,
> A reasonable working IrMC SYNC server (only full phonebook sync support)
> Support for cal and note by just returning nothing.
> ---
> Makefile.am | 3 +
> plugins/irmc.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/main.c | 12 ++-
> src/obex.h | 1 +
> 4 files changed, 502 insertions(+), 2 deletions(-)
> create mode 100644 plugins/irmc.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 73e2f28..ce8c675 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -58,6 +58,9 @@ builtin_sources += plugins/pbap.c plugins/phonebook.h \
> builtin_modules += syncevolution
> builtin_sources += plugins/syncevolution.c
>
> +builtin_modules += irmc
> +builtin_sources += plugins/irmc.c plugins/phonebook.h
> +
> builtin_nodist += plugins/phonebook.c
>
> libexec_PROGRAMS += src/obexd
> diff --git a/plugins/irmc.c b/plugins/irmc.c
> new file mode 100644
> index 0000000..96fd807
> --- /dev/null
> +++ b/plugins/irmc.c
> @@ -0,0 +1,488 @@
> +/*
> + *
> + * OBEX IrMC Sync Server
> + *
> + * Copyright (C) 2010 Marcel Mol <marcel@mesa.nl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <glib.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <arpa/inet.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <openobex/obex.h>
> +#include <openobex/obex_const.h>
> +
> +#include "plugin.h"
> +#include "log.h"
> +#include "obex.h"
> +#include "service.h"
> +#include "phonebook.h"
> +#include "mimetype.h"
> +#include "filesystem.h"
> +#include "dbus.h"
> +
> +#define IRMC_CHANNEL 17
we did have a list of defined RFCOMM channels for various protocols, but
just don't remember where it is.
Johan, we should put that into the doc/ directory actually. Can you dig
up that list and commit it as text file.
> +
> +#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?> \
> +<record> \
> + <attribute id=\"0x0001\"> \
> + <sequence> \
> + <uuid value=\"0x1104\"/> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0004\"> \
> + <sequence> \
> + <sequence> \
> + <uuid value=\"0x0100\"/> \
> + </sequence> \
> + <sequence> \
> + <uuid value=\"0x0003\"/> \
> + <uint8 value=\"%u\" name=\"channel\"/> \
> + </sequence> \
> + <sequence> \
> + <uuid value=\"0x0008\"/> \
> + </sequence> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0009\"> \
> + <sequence> \
> + <sequence> \
> + <uuid value=\"0x1104\"/> \
> + <uint16 value=\"0x0100\" name=\"version\"/> \
> + </sequence> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0100\"> \
> + <text value=\"%s\" name=\"name\"/> \
> + </attribute> \
> + \
> + <attribute id=\"0x0301\"> \
> + <sequence> \
> + <uint8 value=\"0x01\"/> \
> + </sequence> \
> + </attribute> \
> +</record>"
> +
> +
> +struct aparam_header {
> + uint8_t tag;
> + uint8_t len;
> + uint8_t val[0];
> +} __attribute__ ((packed));
> +
> +#define DIDLEN 18
> +struct irmc_session {
> + struct obex_session *os;
> + struct apparam_field *params;
> + uint16_t entries;
> + GString *buffer;
> + char sn[DIDLEN];
> + char did[DIDLEN];
> + char manu[DIDLEN];
> + char model[DIDLEN];
> +};
What is DIDLEN.
> +
> +#define IRMC_TARGET_SIZE 9
> +
> +static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
> + 0x49, 0x52, 0x4d, 0x43, 0x2d, 0x53, 0x59, 0x4e, 0x43 };
> +
> +static void phonebook_size_result(const char *buffer, size_t bufsize,
> + int vcards, int missed, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> +
> + DBG("vcards %d", vcards);
> +
> + irmc->params->maxlistcount = vcards;
> +}
> +
> +static void query_result(const char *buffer, size_t bufsize, int vcards,
> + int missed, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> + const char *s;
> + char *t;
> +
> + DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
> +
> + // first create an empty 'owner' vcard
As Luiz already mentioned, please use /* */ style comments.
> + t = "BEGIN:VCARD\r\n"
> + "VERSION:2.1\r\n"
> + "N:\r\n"
> + "TEL:\r\n"
> + "X-IRMX-LUID:0\r\n"
> + "END:VCARDi\r\n";
You really wanna use const char *t here. And I would even prefer if you
make that one static and global.
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new_len(t, strlen(t));
> + else
> + irmc->buffer = g_string_append_len(irmc->buffer, t, strlen(t));
Why the string_append_len. Just append it it.
> + // loop around buffer and add X-IRMC-LUID attribs
> + s = buffer;
> + while ((t = strstr(s, "UID:")) != NULL) {
> + // add upto UID: into buffer
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + // add UID: line into buffer
> + // Not sure if UID is still needed if X-IRMC-LUID is there
> + s = t;
> + t = strstr(s, "\r\n");
> + t += 2;
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + // add X-IRMC-LUID with same number as UID
> + irmc->buffer = g_string_append_len(irmc->buffer,
> + "X-IRMC-LUID:", 12);
> + s += 4; // point to uid number
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + s = t;
> + }
> + // add remaining bit of buffer
> + irmc->buffer = g_string_append_len(irmc->buffer, s,
> + bufsize - (s-buffer));
> +
> + obex_object_set_io_flags(irmc, G_IO_IN, 0);
> +}
> +
> +static void *irmc_connect(struct obex_session *os, int *err)
> +{
> + struct irmc_session *irmc;
> + struct apparam_field *param;
> +
> + DBG("");
> +
> + manager_register_session(os);
> +
> + irmc = g_new0(struct irmc_session, 1);
> + irmc->os = os;
> +
> + /*
> + * ideally get capabilities info here and use that to define
> + * IrMC DID and SN etc parameters.
> + * For now lets used hostname and some 'random' value
> + */
> + gethostname(irmc->did, DIDLEN);
> + strncpy(irmc->sn, "12345", DIDLEN);
> + strncpy(irmc->manu, "obex", DIDLEN);
> + strncpy(irmc->model, "mymodel", DIDLEN);
> +
> + /*
> + * we need to know the number of contact/cal/nt entries
> + * somewhere so why not do it now.
> + */
> + param = g_new0(struct apparam_field, 1);
> + param->maxlistcount = 0; // to count the number of vcards...
> + param->filter = 0x200085; // UID TEL N VERSION
> + irmc->params = param;
> + phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
> + irmc);
> +
> + if (err)
> + *err = 0;
> +
> + return irmc;
> +}
> +
> +static int irmc_get(struct obex_session *os, obex_object_t *obj,
> + gboolean *stream, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> + const char *type = obex_get_type(os);
> + const char *name = obex_get_name(os);
> + char *path;
> + int ret;
> +
> + DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
> +
> + path = g_strdup(name);
> + *stream = TRUE;
> +
> + ret = obex_get_stream_start(os, path);
> +
> + g_free(path);
> +
> + return ret;
> +}
> +
> +static void irmc_disconnect(struct obex_session *os, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> +
> + DBG("");
> +
> + manager_unregister_session(os);
> +
> + if (irmc->params) {
> + if (irmc->params->searchval)
> + g_free(irmc->params->searchval);
> + g_free(irmc->params);
> + }
> + if (irmc->buffer) {
> + string_free(irmc->buffer);
> + irmc->buffer = NULL;
> + }
> +
> + g_free(irmc);
> +}
> +
> +static int irmc_chkput(struct obex_session *os, void *user_data)
> +{
> + DBG("");
> + /* Reject all PUTs */
> + return -EBADR;
> +}
> +
> +static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
> +{
> + char mybuf[1024];
> +
> + sprintf(mybuf, "MANU:%s\r\n"
> + "MOD:%s\r\n"
> + "SN:%s\r\n"
> + "PB-TYPE-TX:VCARD2.1\r\n",
> + irmc->manu, irmc->model, irmc->sn);
> +
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new(mybuf);
> + else
> + irmc->buffer = g_string_append(irmc->buffer, mybuf);
We have something that is called g_string_append_printf(). Use that one
and not allocate a buf up-front.
> +
> + return irmc;
> +}
> +
> +static void *irmc_open_pb(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + char mybuf[1024];
> + int ret;
> +
> + if (!g_strcmp0(name, ".vcf")) {
> + // how can we tell if the vcard count call already finished?
> + ret = phonebook_pull("telecom/pb.vcf", irmc->params,
> + query_result, irmc);
> + if (ret < 0) {
> + DBG("phonebook_pull failed...");
> + goto fail;
> + }
> + return irmc;
> + }
> +
> + if (!g_strcmp0(name, "/info.log")) {
> + sprintf(mybuf, "Total-Records:%d\r\n"
> + "Maximum-Records:%d\r\n"
> + "DID:%s\r\n",
> + irmc->params->maxlistcount,
> + irmc->params->maxlistcount, irmc->did);
> + }
Same here. A GLib _printf() function is way better usage.
> + else if (!strncmp(name, "/luid/", 6)) {
> + name += 6;
> + if (!g_strcmp0(name, "cc.log"))
> + sprintf(mybuf, "%d\r\n", irmc->params->maxlistcount);
> + else {
> + int l = strlen(name);
> + /* FIXME
> + * reply the same to any *.log so we hopefully force a
> + * full phonebook dump.
> + * Is IEL:2 ok?
> + */
> + if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
> + DBG("changelog request, force whole book");
> + sprintf(mybuf, "SN:%s\r\n"
> + "IEL:2\r\n"
> + "DID:%s\r\n"
> + "Total-Records:%d\r\n"
> + "Maximum-Records:%d\r\n"
> + "*\r\n",
> + irmc->sn, irmc->did,
> + irmc->params->maxlistcount,
> + irmc->params->maxlistcount);
> + }
> + else {
> + ret = -EBADR;
> + goto fail;
> + }
> + }
> + }
> + else {
> + ret = -EBADR;
> + goto fail;
> + }
> +
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new(mybuf);
> + else
> + irmc->buffer = g_string_append(irmc->buffer, mybuf);
> +
> + return irmc;
> +
> +fail:
> + if (err)
> + *err = ret;
> +
> + return NULL;
> +}
> +
> +static void *irmc_open_cal(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + // no suport yet. Just return an empty buffer
> + //cal.vcs
> + DBG("unsupported, returning empty buffer");
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new("");
> + else
> + irmc->buffer = g_string_append(irmc->buffer, "");
> +
> + return irmc;
Empty buffer is empty buffer. Just return it.
> +}
> +
> +static void *irmc_open_nt(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + // no suport yet. Just return an empty buffer
> + //nt.vnt
> + DBG("unsupported, returning empty buffer");
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new("");
> + else
> + irmc->buffer = g_string_append(irmc->buffer, "");
> +
> + return irmc;
Same here.
> +}
> +
> +static void *irmc_open(const char *name, int oflag, mode_t mode,
> + void *context, size_t *size, int *err)
> +{
> + struct irmc_session *irmc = context;
> + int ret;
> + const char *p;
> +
> + DBG("name %s context %p", name, context);
> + if (oflag != O_RDONLY) {
> + ret = -EPERM;
> + goto fail;
> + }
> + if (strncmp(name, "telecom/", 8) != NULL) {
> + ret = -EBADR;
> + goto fail;
> + }
> +
> + p = name + 8;
> + if (!g_strcmp0(p, "devinfo.txt"))
> + return irmc_open_devinfo(irmc, err);
> + else if (!strncmp(p, "pb", 2))
> + return irmc_open_pb(p+2, irmc, err);
> + else if (!strncmp(p, "cal", 3))
> + return irmc_open_cal(p+3, irmc, err);
> + else if (!strncmp(p, "nt", 2))
> + return irmc_open_nt(p+2, irmc, err);
> +
> +fail:
> + if (err)
> + *err = ret;
> +
> + return NULL;
> +}
> +
> +static int irmc_close(void *object)
> +{
> + struct irmc_session *irmc = object;
> +
> + DBG("");
> + if (irmc->buffer) {
> + string_free(irmc->buffer);
> + irmc->buffer = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
> +{
> + struct irmc_session *irmc = object;
> + int len;
> +
> + DBG("buffer %p count %d", irmc->buffer, count);
> + if (!irmc->buffer)
> + return -EAGAIN;
> +
> + *hi = OBEX_HDR_BODY;
> + len = string_read(irmc->buffer, buf, count);
> + DBG("returning %d bytes", len);
> + return len;
> +}
> +
> +static struct obex_mime_type_driver irmc_driver = {
> + .target = IRMC_TARGET,
> + .target_size = IRMC_TARGET_SIZE,
> + .open = irmc_open,
> + .close = irmc_close,
> + .read = irmc_read,
> +};
> +
> +static struct obex_service_driver irmc = {
> + .name = "IRMC Sync server",
> + .service = OBEX_IRMC,
> + .channel = IRMC_CHANNEL,
> + .record = IRMC_RECORD,
> + .target = IRMC_TARGET,
> + .target_size = IRMC_TARGET_SIZE,
> + .connect = irmc_connect,
> + .get = irmc_get,
> + .disconnect = irmc_disconnect,
> + .chkput = irmc_chkput
> +};
> +
> +static int irmc_init(void)
> +{
> + int err;
> +
> + DBG("");
> + err = phonebook_init();
> + if (err < 0)
> + return err;
> +
> + err = obex_mime_type_driver_register(&irmc_driver);
> + if (err < 0)
> + return err;
Don't you need to phonebook_exit() here?
> +
> + return obex_service_driver_register(&irmc);
> +}
Also what happens if this one fails. You do need to clean up.
> +
> +static void irmc_exit(void)
> +{
> + DBG("");
> + obex_service_driver_unregister(&irmc);
> + obex_mime_type_driver_unregister(&irmc_driver);
> + phonebook_exit();
> +}
> +
> +OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
> diff --git a/src/main.c b/src/main.c
> index 649acf9..96523a6 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -83,6 +83,7 @@ static gboolean option_pbap = FALSE;
> static gboolean option_pcsuite = FALSE;
> static gboolean option_symlinks = FALSE;
> static gboolean option_syncevolution = FALSE;
> +static gboolean option_irmc = FALSE;
>
> static gboolean parse_debug(const char *key, const char *value,
> gpointer user_data, GError **error)
> @@ -122,6 +123,8 @@ static GOptionEntry options[] = {
> "Enable PC Suite Services server" },
> { "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
> "Enable OBEX server for SyncEvolution" },
> + { "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
> + "Enable IrMCSync server" },
> { NULL },
> };
Just put irmc before syncevolution. Like nicer ;)
>
> @@ -208,9 +211,10 @@ int main(int argc, char *argv[])
>
> if (option_opp == FALSE && option_ftp == FALSE &&
> option_pbap == FALSE &&
> - option_syncevolution == FALSE) {
> + option_syncevolution == FALSE &&
> + option_irmc == FALSE) {
> fprintf(stderr, "No server selected (use either "
> - "--opp, --ftp, --pbap or --syncevolution)\n");
> + "--opp, --ftp, --pbap --syncevolution or --irmc)\n");
> exit(EXIT_FAILURE);
> }
Same here. Leave syncevolution at the end.
>
> @@ -270,6 +274,10 @@ int main(int argc, char *argv[])
> obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
> FALSE, NULL);
>
> + if (option_irmc == TRUE)
> + obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
> + option_capability);
> +
And here again.
> if (!root_folder_setup(option_root, option_root_setup)) {
> error("Unable to setup root folder %s", option_root);
> exit(EXIT_FAILURE);
> diff --git a/src/obex.h b/src/obex.h
> index 9424b6b..cfe9159 100644
> --- a/src/obex.h
> +++ b/src/obex.h
> @@ -35,6 +35,7 @@
> #define OBEX_PBAP (1 << 4)
> #define OBEX_PCSUITE (1 << 5)
> #define OBEX_SYNCEVOLUTION (1 << 6)
> +#define OBEX_IRMC (1 << 7)
>
And here as well. Just use 1 << 6 for IRMC and move SYNCEVO to 1 << 7.
> #define TARGET_SIZE 16
>
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2] Bluetooth: new sockopt to enter active state when sending data
From: Marcel Holtmann @ 2010-08-12 11:31 UTC (permalink / raw)
To: Emeltchenko Andrei; +Cc: linux-bluetooth
In-Reply-To: <1281448112-2944-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
Hi Andrei,
> Patch adds new socket option to enter active state when sending
> data. Modified version of Fabien Chevalier patch (Sep/2008).
> Discussions:
> http://www.spinics.net/lists/bluez-devel/msg00567.html
> http://www.spinics.net/lists/linux-bluetooth/msg03765.html
>
> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> ---
> include/net/bluetooth/hci_core.h | 1 +
> include/net/bluetooth/l2cap.h | 2 ++
> net/bluetooth/hci_conn.c | 7 ++++++-
> net/bluetooth/l2cap.c | 15 +++++++++++++++
> 4 files changed, 24 insertions(+), 1 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index e42f6ed..477a492 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -178,6 +178,7 @@ struct hci_conn {
> __u8 auth_type;
> __u8 sec_level;
> __u8 power_save;
> + __u8 force_active;
> __u16 disc_timeout;
> unsigned long pend;
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 7c695bf..540cca7 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -74,6 +74,8 @@ struct l2cap_conninfo {
> #define L2CAP_LM_RELIABLE 0x0010
> #define L2CAP_LM_SECURE 0x0020
>
> +#define L2CAP_FORCE_ACTIVE_MODE 0x04
> +
> /* L2CAP command codes */
> #define L2CAP_COMMAND_REJ 0x01
> #define L2CAP_CONN_REQ 0x02
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 800b6b9..11fc44a 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -214,6 +214,8 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst)
> conn->auth_type = HCI_AT_GENERAL_BONDING;
>
> conn->power_save = 1;
> + /* Do not enter active state by default */
> + conn->force_active = 0;
> conn->disc_timeout = HCI_DISCONN_TIMEOUT;
>
> switch (type) {
> @@ -505,7 +507,10 @@ void hci_conn_enter_active_mode(struct hci_conn *conn)
> if (test_bit(HCI_RAW, &hdev->flags))
> return;
>
> - if (conn->mode != HCI_CM_SNIFF || !conn->power_save)
> + if (conn->mode != HCI_CM_SNIFF)
> + goto timer;
> +
> + if (!conn->power_save && !conn->force_active)
> goto timer;
>
> if (!test_and_set_bit(HCI_CONN_MODE_CHANGE_PEND, &conn->pend)) {
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index cf3c407..5328e82 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -1909,6 +1909,15 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us
> l2cap_pi(sk)->force_reliable = (opt & L2CAP_LM_RELIABLE);
> break;
>
> + case L2CAP_FORCE_ACTIVE_MODE:
> + if (get_user(opt, (u32 __user *) optval)) {
> + err = -EFAULT;
> + break;
> + }
> +
> + l2cap_pi(sk)->conn->hcon->force_active = opt;
> + break;
> +
> default:
> err = -ENOPROTOOPT;
> break;
> @@ -2058,6 +2067,12 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname, char __us
>
> break;
>
> + case L2CAP_FORCE_ACTIVE_MODE:
> + if (put_user(l2cap_pi(sk)->conn->hcon->force_active,
> + (u32 __user *) optval))
> + err = -EFAULT;
> + break;
> +
> default:
> err = -ENOPROTOOPT;
> break;
I think that I said this before. Why do you think that adding a new
socket option to something called _old is a good idea? I made it pretty
clear in the code that it is an OLD interface.
I want this on the SOL_BLUETOOTH level with a proper new socket option.
And we might even consider doing it like BT_SECURITY where create the
struct to potentially add other flags later.
So why not doing BT_POWER or something like that.
And in addition you need to store these values on a per L2CAP or RFCOMM
socket. You can not just go ahead and mess inside hci_conn via an L2CAP
socket option.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Marcel Holtmann @ 2010-08-12 11:27 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=GB-QCBmB-2dKoZPrpDU_ZfQupjgD3p0ygv486@mail.gmail.com>
Hi Andrei,
> >> Check that socket sk is not locked in user process before removing
> >> l2cap connection handler.
> >>
> >> krfcommd kernel thread may be preempted with l2cap tasklet which remove
> >> l2cap_conn structure. If krfcommd is in process of sending of RFCOMM reply
> >> (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
> >>
> >> ...
> >> [ 694.175933] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> >> [ 694.184936] pgd = c0004000
> >> [ 694.187683] [00000000] *pgd=00000000
> >> [ 694.191711] Internal error: Oops: 5 [#1] PREEMPT
> >> [ 694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware/hci_h4p/loading
> >> [ 694.260375] CPU: 0 Not tainted (2.6.32.10 #1)
> >> [ 694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
> >> [ 694.270721] LR is at 0xd7017303
> >> ...
> >> [ 694.525085] Backtrace:
> >> [ 694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) from [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
> >> [ 694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044>] (kernel_sendmsg+0x48/0x80)
> >> ...
> >>
> >> Modified version after comments of Gustavo F. Padovan <gustavo@padovan.org>
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >
> > the patch seems to be fine, but I have some extra questions/concerns.
> >
> > Who is now taking care of deleting the channel in this case? I think you
> > need to show that the code flow is still valid.
>
> I have the other version of the I have sent already to ML where I use
> standard l2cap
> timer which will delete channel like the code below:
>
> + /* don't delete l2cap channel if sk is owned by user */
> + if (sock_owned_by_user(sk)) {
> + sk->sk_state = BT_DISCONN;
> + l2cap_sock_clear_timer(sk);
> + l2cap_sock_set_timer(sk, HZ);
> + bh_unlock_sock(sk);
> + return 0;
> + }
>
> > Also the question is how RFCOMM can send this UA or DISC with not
> > locking the socket. The comment on l2cap_chan_del clearly states that
> > the socket must be locked and inside L2CAP we do that. Is RFCOMM maybe
> > at fault here?
>
> when RFCOMM send packets it lock_sock which marks sk as owned
> sk->sk_lock.owned = 1;
> and then can be preempted.
I need a new patch with a proper and most likely lengthy commit message
explaining every single detail here. Since right now you lost me.
Regards
Marcel
^ permalink raw reply
* Re: resending patch to retry reset in case it failed
From: Marcel Holtmann @ 2010-08-12 11:25 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-bluetooth, linux-usb
In-Reply-To: <201008120944.49044.oneukum@suse.de>
Hi Oliver,
> may I ask what happened to this patch?
> Do you take it, want modifications or reject it?
>
> Regards
> Oliver
>
> From af61f0663f095599003290b13ad6fadee1d4fb25 Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oliver@neukum.org>
> Date: Tue, 6 Jul 2010 08:08:42 +0200
> Subject: [PATCH] bluetooth: retry reset for devices that fail
>
> Some devices fail to reset properly at the first attempt to reset
> them under unknown circumstances. Failures can be identified by
> an invalid btaddr. Retry in those cases.
I was pondering about it and haven't really made up my mind with this
stuff. To be honest, I don't like the patch a little bit. I can see your
point why you want it, but the patch is a bit too ugly for my taste.
Tell me which device is causing this and I might be able to reproduce
this and find small less ugly fix. Also keep in mind that the init
routine needs changing to accommodate AMP and LE devices. So we have to
be really careful here.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH] IrMC sync server support
From: Luiz Augusto von Dentz @ 2010-08-12 11:16 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201008121013.o7CADR8r004125@joshua.mesa.nl>
Hi Marcel,
On Tue, Jul 27, 2010 at 8:04 PM, Marcel Mol <marcel@mesa.nl> wrote:
> A reasonable working IrMC SYNC server (only full phonebook sync support)
> Support for cal and note by just returning nothing.
> ---
> Makefile.am | 3 +
> plugins/irmc.c | 488 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> src/main.c | 12 ++-
> src/obex.h | 1 +
> 4 files changed, 502 insertions(+), 2 deletions(-)
> create mode 100644 plugins/irmc.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 73e2f28..ce8c675 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -58,6 +58,9 @@ builtin_sources += plugins/pbap.c plugins/phonebook.h \
> builtin_modules += syncevolution
> builtin_sources += plugins/syncevolution.c
>
> +builtin_modules += irmc
> +builtin_sources += plugins/irmc.c plugins/phonebook.h
> +
> builtin_nodist += plugins/phonebook.c
>
> libexec_PROGRAMS += src/obexd
> diff --git a/plugins/irmc.c b/plugins/irmc.c
> new file mode 100644
> index 0000000..96fd807
> --- /dev/null
> +++ b/plugins/irmc.c
> @@ -0,0 +1,488 @@
> +/*
> + *
> + * OBEX IrMC Sync Server
> + *
> + * Copyright (C) 2010 Marcel Mol <marcel@mesa.nl>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <glib.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <arpa/inet.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +#include <openobex/obex.h>
> +#include <openobex/obex_const.h>
> +
> +#include "plugin.h"
> +#include "log.h"
> +#include "obex.h"
> +#include "service.h"
> +#include "phonebook.h"
> +#include "mimetype.h"
> +#include "filesystem.h"
> +#include "dbus.h"
> +
> +#define IRMC_CHANNEL 17
> +
> +#define IRMC_RECORD "<?xml version=\"1.0\" encoding=\"UTF-8\" ?> \
> +<record> \
> + <attribute id=\"0x0001\"> \
> + <sequence> \
> + <uuid value=\"0x1104\"/> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0004\"> \
> + <sequence> \
> + <sequence> \
> + <uuid value=\"0x0100\"/> \
> + </sequence> \
> + <sequence> \
> + <uuid value=\"0x0003\"/> \
> + <uint8 value=\"%u\" name=\"channel\"/> \
> + </sequence> \
> + <sequence> \
> + <uuid value=\"0x0008\"/> \
> + </sequence> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0009\"> \
> + <sequence> \
> + <sequence> \
> + <uuid value=\"0x1104\"/> \
> + <uint16 value=\"0x0100\" name=\"version\"/> \
> + </sequence> \
> + </sequence> \
> + </attribute> \
> + \
> + <attribute id=\"0x0100\"> \
> + <text value=\"%s\" name=\"name\"/> \
> + </attribute> \
> + \
> + <attribute id=\"0x0301\"> \
> + <sequence> \
> + <uint8 value=\"0x01\"/> \
> + </sequence> \
> + </attribute> \
> +</record>"
> +
> +
> +struct aparam_header {
> + uint8_t tag;
> + uint8_t len;
> + uint8_t val[0];
> +} __attribute__ ((packed));
> +
> +#define DIDLEN 18
> +struct irmc_session {
> + struct obex_session *os;
> + struct apparam_field *params;
> + uint16_t entries;
> + GString *buffer;
> + char sn[DIDLEN];
> + char did[DIDLEN];
> + char manu[DIDLEN];
> + char model[DIDLEN];
> +};
> +
> +#define IRMC_TARGET_SIZE 9
> +
> +static const guint8 IRMC_TARGET[IRMC_TARGET_SIZE] = {
> + 0x49, 0x52, 0x4d, 0x43, 0x2d, 0x53, 0x59, 0x4e, 0x43 };
> +
> +static void phonebook_size_result(const char *buffer, size_t bufsize,
> + int vcards, int missed, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> +
> + DBG("vcards %d", vcards);
> +
> + irmc->params->maxlistcount = vcards;
> +}
> +
> +static void query_result(const char *buffer, size_t bufsize, int vcards,
> + int missed, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> + const char *s;
> + char *t;
> +
> + DBG("bufsize %d vcards %d missed %d", bufsize, vcards, missed);
> +
> + // first create an empty 'owner' vcard
> + t = "BEGIN:VCARD\r\n"
> + "VERSION:2.1\r\n"
> + "N:\r\n"
> + "TEL:\r\n"
> + "X-IRMX-LUID:0\r\n"
> + "END:VCARDi\r\n";
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new_len(t, strlen(t));
> + else
> + irmc->buffer = g_string_append_len(irmc->buffer, t, strlen(t));
> +
> + // loop around buffer and add X-IRMC-LUID attribs
> + s = buffer;
> + while ((t = strstr(s, "UID:")) != NULL) {
> + // add upto UID: into buffer
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + // add UID: line into buffer
> + // Not sure if UID is still needed if X-IRMC-LUID is there
> + s = t;
> + t = strstr(s, "\r\n");
> + t += 2;
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + // add X-IRMC-LUID with same number as UID
> + irmc->buffer = g_string_append_len(irmc->buffer,
> + "X-IRMC-LUID:", 12);
> + s += 4; // point to uid number
> + irmc->buffer = g_string_append_len(irmc->buffer, s, t-s);
> + s = t;
> + }
> + // add remaining bit of buffer
> + irmc->buffer = g_string_append_len(irmc->buffer, s,
> + bufsize - (s-buffer));
> +
> + obex_object_set_io_flags(irmc, G_IO_IN, 0);
> +}
> +
> +static void *irmc_connect(struct obex_session *os, int *err)
> +{
> + struct irmc_session *irmc;
> + struct apparam_field *param;
> +
> + DBG("");
> +
> + manager_register_session(os);
> +
> + irmc = g_new0(struct irmc_session, 1);
> + irmc->os = os;
> +
> + /*
> + * ideally get capabilities info here and use that to define
> + * IrMC DID and SN etc parameters.
> + * For now lets used hostname and some 'random' value
> + */
> + gethostname(irmc->did, DIDLEN);
> + strncpy(irmc->sn, "12345", DIDLEN);
> + strncpy(irmc->manu, "obex", DIDLEN);
> + strncpy(irmc->model, "mymodel", DIDLEN);
> +
> + /*
> + * we need to know the number of contact/cal/nt entries
> + * somewhere so why not do it now.
> + */
> + param = g_new0(struct apparam_field, 1);
> + param->maxlistcount = 0; // to count the number of vcards...
> + param->filter = 0x200085; // UID TEL N VERSION
> + irmc->params = param;
> + phonebook_pull("telecom/pb.vcf", irmc->params, phonebook_size_result,
> + irmc);
> +
> + if (err)
> + *err = 0;
> +
> + return irmc;
> +}
> +
> +static int irmc_get(struct obex_session *os, obex_object_t *obj,
> + gboolean *stream, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> + const char *type = obex_get_type(os);
> + const char *name = obex_get_name(os);
> + char *path;
> + int ret;
> +
> + DBG("name %s type %s irmc %p", name, type ? type : "NA", irmc);
> +
> + path = g_strdup(name);
> + *stream = TRUE;
> +
> + ret = obex_get_stream_start(os, path);
> +
> + g_free(path);
> +
> + return ret;
> +}
> +
> +static void irmc_disconnect(struct obex_session *os, void *user_data)
> +{
> + struct irmc_session *irmc = user_data;
> +
> + DBG("");
> +
> + manager_unregister_session(os);
> +
> + if (irmc->params) {
> + if (irmc->params->searchval)
> + g_free(irmc->params->searchval);
> + g_free(irmc->params);
> + }
> + if (irmc->buffer) {
> + string_free(irmc->buffer);
> + irmc->buffer = NULL;
> + }
> +
> + g_free(irmc);
> +}
> +
> +static int irmc_chkput(struct obex_session *os, void *user_data)
> +{
> + DBG("");
> + /* Reject all PUTs */
> + return -EBADR;
> +}
> +
> +static void *irmc_open_devinfo(struct irmc_session *irmc, int *err)
> +{
> + char mybuf[1024];
> +
> + sprintf(mybuf, "MANU:%s\r\n"
> + "MOD:%s\r\n"
> + "SN:%s\r\n"
> + "PB-TYPE-TX:VCARD2.1\r\n",
> + irmc->manu, irmc->model, irmc->sn);
> +
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new(mybuf);
> + else
> + irmc->buffer = g_string_append(irmc->buffer, mybuf);
> +
> + return irmc;
> +}
> +
> +static void *irmc_open_pb(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + char mybuf[1024];
> + int ret;
> +
> + if (!g_strcmp0(name, ".vcf")) {
> + // how can we tell if the vcard count call already finished?
> + ret = phonebook_pull("telecom/pb.vcf", irmc->params,
> + query_result, irmc);
> + if (ret < 0) {
> + DBG("phonebook_pull failed...");
> + goto fail;
> + }
> + return irmc;
> + }
> +
> + if (!g_strcmp0(name, "/info.log")) {
> + sprintf(mybuf, "Total-Records:%d\r\n"
> + "Maximum-Records:%d\r\n"
> + "DID:%s\r\n",
> + irmc->params->maxlistcount,
> + irmc->params->maxlistcount, irmc->did);
> + }
> + else if (!strncmp(name, "/luid/", 6)) {
> + name += 6;
> + if (!g_strcmp0(name, "cc.log"))
> + sprintf(mybuf, "%d\r\n", irmc->params->maxlistcount);
> + else {
> + int l = strlen(name);
> + /* FIXME
> + * reply the same to any *.log so we hopefully force a
> + * full phonebook dump.
> + * Is IEL:2 ok?
> + */
> + if (l > 4 && !g_strcmp0(name + l - 4, ".log")) {
> + DBG("changelog request, force whole book");
> + sprintf(mybuf, "SN:%s\r\n"
> + "IEL:2\r\n"
> + "DID:%s\r\n"
> + "Total-Records:%d\r\n"
> + "Maximum-Records:%d\r\n"
> + "*\r\n",
> + irmc->sn, irmc->did,
> + irmc->params->maxlistcount,
> + irmc->params->maxlistcount);
> + }
> + else {
> + ret = -EBADR;
> + goto fail;
> + }
> + }
> + }
> + else {
> + ret = -EBADR;
> + goto fail;
> + }
> +
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new(mybuf);
> + else
> + irmc->buffer = g_string_append(irmc->buffer, mybuf);
> +
> + return irmc;
> +
> +fail:
> + if (err)
> + *err = ret;
> +
> + return NULL;
> +}
> +
> +static void *irmc_open_cal(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + // no suport yet. Just return an empty buffer
> + //cal.vcs
> + DBG("unsupported, returning empty buffer");
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new("");
> + else
> + irmc->buffer = g_string_append(irmc->buffer, "");
> +
> + return irmc;
> +}
> +
> +static void *irmc_open_nt(const char *name,
> + struct irmc_session *irmc, int *err)
> +{
> + // no suport yet. Just return an empty buffer
> + //nt.vnt
> + DBG("unsupported, returning empty buffer");
> + if (!irmc->buffer)
> + irmc->buffer = g_string_new("");
> + else
> + irmc->buffer = g_string_append(irmc->buffer, "");
> +
> + return irmc;
> +}
> +
> +static void *irmc_open(const char *name, int oflag, mode_t mode,
> + void *context, size_t *size, int *err)
> +{
> + struct irmc_session *irmc = context;
> + int ret;
> + const char *p;
> +
> + DBG("name %s context %p", name, context);
> + if (oflag != O_RDONLY) {
> + ret = -EPERM;
> + goto fail;
> + }
> + if (strncmp(name, "telecom/", 8) != NULL) {
> + ret = -EBADR;
> + goto fail;
> + }
> +
> + p = name + 8;
> + if (!g_strcmp0(p, "devinfo.txt"))
> + return irmc_open_devinfo(irmc, err);
> + else if (!strncmp(p, "pb", 2))
> + return irmc_open_pb(p+2, irmc, err);
> + else if (!strncmp(p, "cal", 3))
> + return irmc_open_cal(p+3, irmc, err);
> + else if (!strncmp(p, "nt", 2))
> + return irmc_open_nt(p+2, irmc, err);
> +
> +fail:
> + if (err)
> + *err = ret;
> +
> + return NULL;
> +}
> +
> +static int irmc_close(void *object)
> +{
> + struct irmc_session *irmc = object;
> +
> + DBG("");
> + if (irmc->buffer) {
> + string_free(irmc->buffer);
> + irmc->buffer = NULL;
> + }
> +
> + return 0;
> +}
> +
> +static ssize_t irmc_read(void *object, void *buf, size_t count, uint8_t *hi)
> +{
> + struct irmc_session *irmc = object;
> + int len;
> +
> + DBG("buffer %p count %d", irmc->buffer, count);
> + if (!irmc->buffer)
> + return -EAGAIN;
> +
> + *hi = OBEX_HDR_BODY;
> + len = string_read(irmc->buffer, buf, count);
> + DBG("returning %d bytes", len);
> + return len;
> +}
> +
> +static struct obex_mime_type_driver irmc_driver = {
> + .target = IRMC_TARGET,
> + .target_size = IRMC_TARGET_SIZE,
> + .open = irmc_open,
> + .close = irmc_close,
> + .read = irmc_read,
> +};
> +
> +static struct obex_service_driver irmc = {
> + .name = "IRMC Sync server",
> + .service = OBEX_IRMC,
> + .channel = IRMC_CHANNEL,
> + .record = IRMC_RECORD,
> + .target = IRMC_TARGET,
> + .target_size = IRMC_TARGET_SIZE,
> + .connect = irmc_connect,
> + .get = irmc_get,
> + .disconnect = irmc_disconnect,
> + .chkput = irmc_chkput
> +};
> +
> +static int irmc_init(void)
> +{
> + int err;
> +
> + DBG("");
> + err = phonebook_init();
> + if (err < 0)
> + return err;
> +
> + err = obex_mime_type_driver_register(&irmc_driver);
> + if (err < 0)
> + return err;
> +
> + return obex_service_driver_register(&irmc);
> +}
> +
> +static void irmc_exit(void)
> +{
> + DBG("");
> + obex_service_driver_unregister(&irmc);
> + obex_mime_type_driver_unregister(&irmc_driver);
> + phonebook_exit();
> +}
> +
> +OBEX_PLUGIN_DEFINE(irmc, irmc_init, irmc_exit)
> diff --git a/src/main.c b/src/main.c
> index 649acf9..96523a6 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -83,6 +83,7 @@ static gboolean option_pbap = FALSE;
> static gboolean option_pcsuite = FALSE;
> static gboolean option_symlinks = FALSE;
> static gboolean option_syncevolution = FALSE;
> +static gboolean option_irmc = FALSE;
>
> static gboolean parse_debug(const char *key, const char *value,
> gpointer user_data, GError **error)
> @@ -122,6 +123,8 @@ static GOptionEntry options[] = {
> "Enable PC Suite Services server" },
> { "syncevolution", 'e', 0, G_OPTION_ARG_NONE, &option_syncevolution,
> "Enable OBEX server for SyncEvolution" },
> + { "irmc", 'i', 0, G_OPTION_ARG_NONE, &option_irmc,
> + "Enable IrMCSync server" },
> { NULL },
> };
>
> @@ -208,9 +211,10 @@ int main(int argc, char *argv[])
>
> if (option_opp == FALSE && option_ftp == FALSE &&
> option_pbap == FALSE &&
> - option_syncevolution == FALSE) {
> + option_syncevolution == FALSE &&
> + option_irmc == FALSE) {
> fprintf(stderr, "No server selected (use either "
> - "--opp, --ftp, --pbap or --syncevolution)\n");
> + "--opp, --ftp, --pbap --syncevolution or --irmc)\n");
> exit(EXIT_FAILURE);
> }
>
> @@ -270,6 +274,10 @@ int main(int argc, char *argv[])
> obex_server_init(OBEX_SYNCEVOLUTION, NULL, TRUE, FALSE,
> FALSE, NULL);
>
> + if (option_irmc == TRUE)
> + obex_server_init(OBEX_IRMC, NULL, TRUE, FALSE, FALSE,
> + option_capability);
> +
> if (!root_folder_setup(option_root, option_root_setup)) {
> error("Unable to setup root folder %s", option_root);
> exit(EXIT_FAILURE);
> diff --git a/src/obex.h b/src/obex.h
> index 9424b6b..cfe9159 100644
> --- a/src/obex.h
> +++ b/src/obex.h
> @@ -35,6 +35,7 @@
> #define OBEX_PBAP (1 << 4)
> #define OBEX_PCSUITE (1 << 5)
> #define OBEX_SYNCEVOLUTION (1 << 6)
> +#define OBEX_IRMC (1 << 7)
>
> #define TARGET_SIZE 16
>
> --
> 1.7.2
Looks pretty good, but you need to fix those c++ comments (//) please
use /*, also the you don't necessary need to abstract the file
devinfo.txt so platform can just write their own stuff there, if that
is not sufficient than we can thing of supporting scripts like we do
with x-obex/capability but I don't think we really need it since the
device info should be static, right?
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] ebook backend: separate concatenated vcards with \r\n.
From: Johan Hedberg @ 2010-08-12 11:15 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201008121026.o7CAQDVU005280@joshua.mesa.nl>
Hi Marcel,
On Thu, Aug 12, 2010, Marcel Mol wrote:
> The END:VCARD is not ended with \r\n for single vcards.
> In a phonebook pull request vcards are concatenated as one
> big blob. Carkits seem to choke on the fact that an END:VCARD
> is directly followed by BEGIN:VCARD without \r\n separation.
> (as observed by many n900 users).
> ---
> plugins/phonebook-ebook.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
This one is also upstream now. Thanks.
Johan
^ permalink raw reply
* Re: [PATCH] Cosmetic: fix comment typos/indents
From: Johan Hedberg @ 2010-08-12 11:15 UTC (permalink / raw)
To: Marcel Mol; +Cc: linux-bluetooth
In-Reply-To: <201008121026.o7CAQ00K005274@joshua.mesa.nl>
Hi Marcel,
On Thu, Aug 12, 2010, Marcel Mol wrote:
> ---
> plugins/pbap.c | 2 +-
> plugins/phonebook-ebook.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* Re: [PATCH] Use enum from vcard.h for phone types
From: Johan Hedberg @ 2010-08-12 11:13 UTC (permalink / raw)
To: Radoslaw Jablonski; +Cc: linux-bluetooth
In-Reply-To: <1281599353-27811-1-git-send-email-ext-jablonski.radoslaw@nokia.com>
Hi Radek,
On Thu, Aug 12, 2010, Radoslaw Jablonski wrote:
> Removed redundant declarations for phone types. Now using values from
> phonebook_number_type declared in vcard.h.
> ---
> plugins/phonebook-tracker.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
The patch has been applied and pushed upstream. Thanks.
Johan
^ permalink raw reply
* [PATCH] ebook backend: separate concatenated vcards with \r\n.
From: Marcel Mol @ 2010-08-12 10:20 UTC (permalink / raw)
To: linux-bluetooth
The END:VCARD is not ended with \r\n for single vcards.
In a phonebook pull request vcards are concatenated as one
big blob. Carkits seem to choke on the fact that an END:VCARD
is directly followed by BEGIN:VCARD without \r\n separation.
(as observed by many n900 users).
---
plugins/phonebook-ebook.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index ad40be3..da68ac0 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -175,8 +175,11 @@ static void ebookpull_cb(EBook *book, EBookStatus estatus, GList *contacts,
data->params->filter);
string = g_string_append(string, vcard);
+ string = g_string_append(string, "\r\n");
g_free(vcard);
}
+ DBG("collected %d vcards", count);
+
done:
data->cb(string->str, string->len, count, 0, data->user_data);
--
1.7.2
^ permalink raw reply related
* [PATCH] Cosmetic: fix comment typos/indents
From: Marcel Mol @ 2010-08-12 10:17 UTC (permalink / raw)
To: linux-bluetooth
---
plugins/pbap.c | 2 +-
plugins/phonebook-ebook.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/plugins/pbap.c b/plugins/pbap.c
index c008e95..948f9df 100644
--- a/plugins/pbap.c
+++ b/plugins/pbap.c
@@ -921,7 +921,7 @@ static int pbap_init(void)
err = obex_service_driver_register(&pbap);
if (err < 0)
- goto fail_pbap_reg;
+ goto fail_pbap_reg;
return 0;
diff --git a/plugins/phonebook-ebook.c b/plugins/phonebook-ebook.c
index 1598d5f..ad40be3 100644
--- a/plugins/phonebook-ebook.c
+++ b/plugins/phonebook-ebook.c
@@ -152,7 +152,7 @@ static void ebookpull_cb(EBook *book, EBookStatus estatus, GList *contacts,
}
/*
- * When MaxListCount s zero, PCE wants to know the number of used
+ * When MaxListCount is zero, PCE wants to know the number of used
* indexes in the phonebook of interest. All other parameters that
* may be present in the request shall be ignored.
*/
--
1.7.2
^ permalink raw reply related
* Re: [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Andrei Emeltchenko @ 2010-08-12 9:49 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1278429662.2789.52.camel@localhost.localdomain>
Hi Marcel,
On Tue, Jul 6, 2010 at 6:21 PM, Marcel Holtmann <marcel@holtmann.org> wrote=
:
> Hi Andrei,
>
>> Check that socket sk is not locked in user process before removing
>> l2cap connection handler.
>>
>> krfcommd kernel thread may be preempted with l2cap tasklet which remove
>> l2cap_conn structure. If krfcommd is in process of sending of RFCOMM rep=
ly
>> (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
>>
>> ...
>> [ =A0694.175933] Unable to handle kernel NULL pointer dereference at vir=
tual address 00000000
>> [ =A0694.184936] pgd =3D c0004000
>> [ =A0694.187683] [00000000] *pgd=3D00000000
>> [ =A0694.191711] Internal error: Oops: 5 [#1] PREEMPT
>> [ =A0694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware=
/hci_h4p/loading
>> [ =A0694.260375] CPU: 0 =A0 =A0Not tainted =A0(2.6.32.10 #1)
>> [ =A0694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap]
>> [ =A0694.270721] LR is at 0xd7017303
>> ...
>> [ =A0694.525085] Backtrace:
>> [ =A0694.527587] [<bf266be0>] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) fro=
m [<c02f2cc8>] (sock_sendmsg+0xb8/0xd8)
>> [ =A0694.537292] [<c02f2c10>] (sock_sendmsg+0x0/0xd8) from [<c02f3044>] =
(kernel_sendmsg+0x48/0x80)
>> ...
>>
>> Modified version after comments of Gustavo F. Padovan <gustavo@padovan.o=
rg>
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>
> the patch seems to be fine, but I have some extra questions/concerns.
>
> Who is now taking care of deleting the channel in this case? I think you
> need to show that the code flow is still valid.
I have the other version of the I have sent already to ML where I use
standard l2cap
timer which will delete channel like the code below:
+ /* don't delete l2cap channel if sk is owned by user */
+ if (sock_owned_by_user(sk)) {
+ sk->sk_state =3D BT_DISCONN;
+ l2cap_sock_clear_timer(sk);
+ l2cap_sock_set_timer(sk, HZ);
+ bh_unlock_sock(sk);
+ return 0;
+ }
> Also the question is how RFCOMM can send this UA or DISC with not
> locking the socket. The comment on l2cap_chan_del clearly states that
> the socket must be locked and inside L2CAP we do that. Is RFCOMM maybe
> at fault here?
when RFCOMM send packets it lock_sock which marks sk as owned
sk->sk_lock.owned =3D 1;
and then can be preempted.
Regards,
Andrei
^ permalink raw reply
* [PATCH] Use enum from vcard.h for phone types
From: Radoslaw Jablonski @ 2010-08-12 7:49 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Radoslaw Jablonski
Removed redundant declarations for phone types. Now using values from
phonebook_number_type declared in vcard.h.
---
plugins/phonebook-tracker.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/plugins/phonebook-tracker.c b/plugins/phonebook-tracker.c
index 9cd82e7..8300fc7 100644
--- a/plugins/phonebook-tracker.c
+++ b/plugins/phonebook-tracker.c
@@ -44,8 +44,6 @@
#define TRACKER_DEFAULT_CONTACT_ME "http://www.semanticdesktop.org/ontologies/2007/03/22/nco#default-contact-me"
#define CONTACTS_ID_COL 19
-#define PHONE_ID_HOME 0
-#define PHONE_ID_WORK 3
#define CONTACTS_QUERY_ALL \
"SELECT nco:phoneNumber(?h) nco:fullname(?c) " \
@@ -697,8 +695,8 @@ add_entry:
add_numbers:
/* Adding phone numbers to contact struct */
- add_phone_number(contact, reply[0], PHONE_ID_HOME);
- add_phone_number(contact, reply[8], PHONE_ID_WORK);
+ add_phone_number(contact, reply[0], TEL_TYPE_HOME);
+ add_phone_number(contact, reply[8], TEL_TYPE_WORK);
DBG("contact %p", contact);
--
1.7.0.4
^ permalink raw reply related
* resending patch to retry reset in case it failed
From: Oliver Neukum @ 2010-08-12 7:44 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, linux-usb
Hi Marcel,
may I ask what happened to this patch?
Do you take it, want modifications or reject it?
Regards
Oliver
=46rom af61f0663f095599003290b13ad6fadee1d4fb25 Mon Sep 17 00:00:00 2001
=46rom: Oliver Neukum <oliver@neukum.org>
Date: Tue, 6 Jul 2010 08:08:42 +0200
Subject: [PATCH] bluetooth: retry reset for devices that fail
Some devices fail to reset properly at the first attempt to reset
them under unknown circumstances. Failures can be identified by
an invalid btaddr. Retry in those cases.
Signed-off-by: Oliver Neukum <oneukum@suse.de>
=2D--
net/bluetooth/hci_core.c | 48 ++++++++++++++++++++++++++----------------=
=2D--
1 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 2f768de..c47f73d 100644
=2D-- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -180,11 +180,17 @@ static void hci_reset_req(struct hci_dev *hdev, unsig=
ned long opt)
hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
}
=20
+static int verify_valid_bdaddr(struct hci_dev *hdev)
+{
+ return !bacmp(&hdev->bdaddr, BDADDR_ANY);
+}
+
static void hci_init_req(struct hci_dev *hdev, unsigned long opt)
{
struct sk_buff *skb;
__le16 param;
__u8 flt_type;
+ int retries =3D 4;
=20
BT_DBG("%s %ld", hdev->name, opt);
=20
@@ -202,33 +208,35 @@ static void hci_init_req(struct hci_dev *hdev, unsign=
ed long opt)
=20
/* Mandatory initialization */
=20
=2D /* Reset */
=2D if (!test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
=2D hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
+ do {
+ /* Reset */
+ if (!test_bit(HCI_QUIRK_NO_RESET, &hdev->quirks))
+ hci_send_cmd(hdev, HCI_OP_RESET, 0, NULL);
=20
=2D /* Read Local Supported Features */
=2D hci_send_cmd(hdev, HCI_OP_READ_LOCAL_FEATURES, 0, NULL);
+ /* Read Local Supported Features */
+ hci_send_cmd(hdev, HCI_OP_READ_LOCAL_FEATURES, 0, NULL);
=20
=2D /* Read Local Version */
=2D hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
+ /* Read Local Version */
+ hci_send_cmd(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL);
=20
=2D /* Read Buffer Size (ACL mtu, max pkt, etc.) */
=2D hci_send_cmd(hdev, HCI_OP_READ_BUFFER_SIZE, 0, NULL);
+ /* Read Buffer Size (ACL mtu, max pkt, etc.) */
+ hci_send_cmd(hdev, HCI_OP_READ_BUFFER_SIZE, 0, NULL);
=20
#if 0
=2D /* Host buffer size */
=2D {
=2D struct hci_cp_host_buffer_size cp;
=2D cp.acl_mtu =3D cpu_to_le16(HCI_MAX_ACL_SIZE);
=2D cp.sco_mtu =3D HCI_MAX_SCO_SIZE;
=2D cp.acl_max_pkt =3D cpu_to_le16(0xffff);
=2D cp.sco_max_pkt =3D cpu_to_le16(0xffff);
=2D hci_send_cmd(hdev, HCI_OP_HOST_BUFFER_SIZE, sizeof(cp), &cp);
=2D }
+ /* Host buffer size */
+ {
+ struct hci_cp_host_buffer_size cp;
+ cp.acl_mtu =3D cpu_to_le16(HCI_MAX_ACL_SIZE);
+ cp.sco_mtu =3D HCI_MAX_SCO_SIZE;
+ cp.acl_max_pkt =3D cpu_to_le16(0xffff);
+ cp.sco_max_pkt =3D cpu_to_le16(0xffff);
+ hci_send_cmd(hdev, HCI_OP_HOST_BUFFER_SIZE, sizeof(cp), &cp);
+ }
#endif
=20
=2D /* Read BD Address */
=2D hci_send_cmd(hdev, HCI_OP_READ_BD_ADDR, 0, NULL);
+ /* Read BD Address */
+ hci_send_cmd(hdev, HCI_OP_READ_BD_ADDR, 0, NULL);
+ } while(!verify_valid_bdaddr(hdev) && retries--);
=20
/* Read Class of Device */
hci_send_cmd(hdev, HCI_OP_READ_CLASS_OF_DEV, 0, NULL);
=2D-=20
1.7.1
^ permalink raw reply
* Re: why is hciops a plugin ?
From: Johan Hedberg @ 2010-08-12 3:07 UTC (permalink / raw)
To: Pavan Savoy; +Cc: linux-bluetooth@vger.kernel.org, marcel
In-Reply-To: <AANLkTimku3Obrb=4KFjD5mapWpYQ0__4UZmzbfadu3WM@mail.gmail.com>
Hi,
On Wed, Aug 11, 2010, Pavan Savoy wrote:
> When you say raw HCI access do you mean doing the
> socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI) and bind/ioctl, writev+poll/read
> lacks few things ?
> but all of these are in lib/hci isn't it ?
Yes, that's the type of socket that I meant. Direct access to the HCI
messages would largely go away from the userspace side. Those
libbluetooth functions you're referring to are just convenience wrappers
for sending HCI commands through raw HCI sockets.
> So even if it was netlink only something like hci_open_dev() would
> have changed to socket(PF_NETLINK, SOCK_RAW, BT_NETLINK ); or
> something right ?
I'm not really familiar enough with netlink to comment on this. Marcel
(whose idea it originally was) would have to comment.
> any pointers out there ? references for such things ?
> I am just curious, don;t want anything specific ...
Right now, not really. The only stack internal messages there are at the
moment are things like HCI_DEV_REG, HCI_DEV_UNREG, HCI_DEV_UP and
HCI_DEV_DOWN and they only go one way (kernel->userspace). This category
of messages will grow in the future and it'll be possible to send them
both ways.
Some HCI messages for which it's already clear now that there will be a
benefit from a higher level abstraction on the userspace side are things
like name resolving and pairing related requests. Also, if I understood
correctly from Marcel, removing userspace processing of HCI events will
result in a considreable reduction of context switches since there wont
anymore be a promiscuous userspace-side socket that needs to
handle/filter all HCI data (Marcel, please correct me if this is
inaccurate or wrong).
Johan
^ permalink raw reply
* Re: QuiC AMP development
From: sober song @ 2010-08-12 2:17 UTC (permalink / raw)
To: Gustavo F. Padovan; +Cc: Mat Martineau, Ron Shaffer, linux-bluetooth
In-Reply-To: <20100811031551.GA8985@vigoh>
Hi Shaffer/Peter
static inline int send_a2mp_cmd(struct amp_mgr *mgr, u8 ident, u8
code, u16 len, void *data) {
struct a2mp_cmd_hdr *hdr;
int plen;
u8 *buf;
BT_DBG("ident %d code %d", ident, code);
plen = sizeof(*hdr) + len;
buf = kzalloc(plen, GFP_ATOMIC);
if (!buf)
return -ENOMEM;
hdr = (struct a2mp_cmd_hdr *) buf;
hdr->code = code;
hdr->ident = ident;
hdr->len = cpu_to_le16(len);
buf += sizeof(*hdr);
memcpy(buf, data, len);
return send_a2mp(mgr->a2mp_sock, (u8 *) hdr, plen);
}
I see that here have a malloc, but i don't see free, doesn't it cause memleak?
Regards
sober
On 8/11/10, Gustavo F. Padovan <gustavo@padovan.org> wrote:
> Hi Mat,
>
> * Mat Martineau <mathewm@codeaurora.org> [2010-08-10 19:55:34 -0700]:
>
>>
>> Gustavo -
>>
>> On Tue, 10 Aug 2010, Gustavo F. Padovan wrote:
>>
>> >Hi all,
>> >
>> >* Ron Shaffer <rshaffer@codeaurora.org> [2010-08-08 21:43:36 -0500]:
>> >
>> >>As requested in today's summit discussions, here's a link that can be
>> >>used to inspect the code we've done to add AMP support on the latest
>> >>next tree.
>> >>
>> >>https://www.codeaurora.org/gitweb/quic/bluetooth/?p=bluetooth-next-2.6.git;a=summary
>> >>branch pk-upstream
>> >
>> >
>> >I'll put here some comments I have about some L2CAP patches:
>>
>> Thanks for taking the time to look over all these changes!
>>
>> >
>> >
>> >+static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16
>> > size)
>> >+{
>> >+ u16 allocSize;
>> >
>> >No capital letter here. Do alloc_size. Check the rest of your code for
>> > similar
>> >stuff.
>>
>> Yes, someone caught that in our internal code review too. I think
>> this was the only one.
>>
>> >
>> >
>> > u8 event;
>> > struct sk_buff *skb;
>> >};
>> >+
>> >#endif /* __AMP_H */
>> >diff --git a/net/bluetooth/amp.c b/net/bluetooth/amp.c
>> >index f43d7d8..16e74f6 100644
>> >--- a/net/bluetooth/amp.c
>> >+++ b/net/bluetooth/amp.c
>> >@@ -40,6 +40,7 @@ static struct workqueue_struct *amp_workqueue;
>> >LIST_HEAD(amp_mgr_list);
>> >DEFINE_RWLOCK(amp_mgr_list_lock);
>> >
>> >+
>> >static void remove_amp_mgr(struct amp_mgr *rem)
>> >{
>> > struct amp_mgr *mgr = NULL;
>> >
>> >Do not add new lines random places into your patch. Check you code for
>> > others
>> >things like that. Also check for lines overstepping the column 80.
>>
>> We definitely know we have work to do to clean up these patches and
>> add proper commit messages. Please be assured that we'll have these
>> more tidied up before posting patches to linux-bluetooth.
>
> ;)
>
>>
>> >
>> >
>> >+struct l2cap_enhanced_hdr {
>> >+ __le16 len;
>> >+ __le16 cid;
>> >+ __le16 control;
>> >+} __attribute__ ((packed));
>> >+#define L2CAP_ENHANCED_HDR_SIZE 6
>> >
>> >Get ride off this struct, that actually doesn't help. We tried that
>> > before and
>> >Marcel and I agreed that it does not help.
>>
>> If you guys don't like it, I'll take it out.
>>
>> >
>> >- if (memcpy_fromiovec(skb_put(*frag, count), msg->msg_iov,
>> > count))
>> >+
>> >+ if (memcpy_fromiovec(skb_put(*frag, count),
>> >+ msg->msg_iov, count))
>> > return -EFAULT;
>> >
>> >Avoid changes like that in your patches, you are just breaking a line
>> > here. If
>> >you want to do that do in a separeted patch.
>> >
>> >
>> >
>> >+/* L2CAP ERTM / Streaming extra field lengths */
>> >+#define L2CAP_FCS_SIZE 2
>> >
>> >Separated patch for that, so then you replace 2 for L2CAP_FCS_SIZE in one
>> >patch, instead of doing that in many patches like you are doing now. The
>> > same
>> >goes for L2CAP_SDULEN_SIZE.
>>
>> Yes, I already split that commit up to send the "RFC" patch set
>> earlier today. Unfortunately I missed that commit when generating
>> my patches :(
>>
>>
>> >-/* L2CAP Supervisory Function */
>> >+/* L2CAP Supervisory Frame Types */
>> >+#define L2CAP_SFRAME_RR 0x00
>> >+#define L2CAP_SFRAME_REJ 0x01
>> >+#define L2CAP_SFRAME_RNR 0x02
>> >+#define L2CAP_SFRAME_SREJ 0x03
>> >#define L2CAP_SUPER_RCV_READY 0x0000
>> >#define L2CAP_SUPER_REJECT 0x0004
>> >#define L2CAP_SUPER_RCV_NOT_READY 0x0008
>> >#define L2CAP_SUPER_SELECT_REJECT 0x000C
>> >
>> >/* L2CAP Segmentation and Reassembly */
>> >+#define L2CAP_SAR_UNSEGMENTED 0x00
>> >+#define L2CAP_SAR_START 0x01
>> >+#define L2CAP_SAR_END 0x02
>> >+#define L2CAP_SAR_CONTINUE 0x03
>> >#define L2CAP_SDU_UNSEGMENTED 0x0000
>> >#define L2CAP_SDU_START 0x4000
>> >#define L2CAP_SDU_END 0x8000
>> >
>> >What is that? we already have such defines.
>>
>> I was trying to not break existing code, while also creating
>> constants that were useful for modified code that will work with
>> either extended or enhanced headers. The new values are important,
>> but I agree that this is not the way to introduce the changes. See
>> below for more detail.
>>
>> >commit 162e6de6d5c11b8ffea91a9fa2d597340afdeb6e
>> >Author: Mat Martineau <mathewm@codeaurora.org>
>> >Date: Thu Aug 5 16:59:46 2010 -0700
>> >
>> > Bluetooth: Remove unused L2CAP code.
>> >
>> >net/bluetooth/l2cap.c | 1104
>> > +------------------------------------------------
>> >1 files changed, 15 insertions(+), 1089 deletions(-)
>> >
>> >That's not acceptable, we have to modify the existent base code and make
>> > it
>> >work with the new features, instead writing a new one and then switch to
>> > it.
>>
>> When setting up the commits for this git branch, I had to pick
>> between two approaches:
>>
>> * Build up a modified state machine in parallel, so the switchover
>> could happen in one patch. The code would compile and run after
>> every patch.
>>
>> * Or, start modifying the state machine piece by piece. Until all
>> of the modifications were done, ERTM would not work.
>>
>> I don't think my approach worked out well (mostly because it doesn't
>> preserve history adequately, and doesn't make it clear where code
>> has been modified instead of newly written). However, it's what we
>> had to share coming in to the Bluetooth summit, and we felt it was
>> very important to share it. I want to refine the approach to these
>> patches to put them in some acceptable form, so lets talk about the
>> best way to do that.
>
> I see. But now that they are public we can talk about and put them in
> shape for the merge into the mainline.
> I think it's possible to add AMP stuff without break ERTM (and we have
> to take care about that, because now ERTM pass all PTS tests). We can
> arrange your commits to not break ERTM any time.
>
>>
>>
>> >commit 09850f68219572288fe62a59235fda3d2629c7b2
>> >Author: Peter Krystad <pkrystad@codeaurora.org>
>> >Date: Wed Aug 4 16:55:11 2010 -0700
>> >
>> > Rename l2cap.c to el2cap.c.
>> >
>> > Necessary before additional files can be added to l2cap module.
>> > A module cannot contain a file with the same name as the module.
>> >
>> >We don't neeed that. We might split l2cap.c into smaller chunks before
>> > merge
>> >all the AMP stuff into it, so we won't have the module name problem
>> > anymore.
>>
>> There is a technical reason for this. We changed the Makefile to
>> create l2cap.ko from two source files - one for L2CAP and one for
>> AMP. However, the build system fails if you try to link l2cap.c and
>> amp.c in to l2cap.ko. We had to come up with some other name for
>> l2cap.c in order to keep the module name the same. There's probably
>> a better choice than el2cap.c - any suggestions are welcome.
>
> Yeah, I know, that's why the l2cap.c split will help this. Marcel
> told that on the meeting Sunday.
>
> --
> Gustavo F. Padovan
> http://padovan.org
> --
> 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
>
^ permalink raw reply
* Re: why is hciops a plugin ?
From: Pavan Savoy @ 2010-08-11 19:41 UTC (permalink / raw)
To: Pavan Savoy, linux-bluetooth@vger.kernel.org
In-Reply-To: <20100811021610.GA8306@jh-x301>
On Tue, Aug 10, 2010 at 9:16 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi,
>
> On Tue, Aug 10, 2010, Pavan Savoy wrote:
>> Any reason behind making hciops a plugin and bluetoothd making use of
>> that plugin, wasn't everything straight up in hcid before ?
>
> We're planning on redoing the kernel-userspace interface and in the long
> run the traditional raw HCI access wont exist anymore in userspace. In
> order to accommodate both old and new kernels in a clean way the
> interface towards the kernel needs to be easily interchangable, which is
> what hciops is trying to do (it's just a first step in that direction
> though). One of the drivers for this change is the need to have the
> security logic in one place instead of it being split between kernel and
> userspace (this has caused all sorts of trouble with SSP for us).
When you say raw HCI access do you mean doing the
socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI) and bind/ioctl, writev+poll/read
lacks few things ?
but all of these are in lib/hci isn't it ?
> Originally the plan for the new kernel interface was netlink but now the
So even if it was netlink only something like hci_open_dev() would
have changed to
socket(PF_NETLINK, SOCK_RAW, BT_NETLINK ); or something right ?
> idea is to simply extend the stack internal messages of raw HCI sockets
> with a more complete two-way protocol between the kernel and userspace.
> You should (hopefully) be seeing more patches for this still during this
> year.
any pointers out there ? references for such things ?
I am just curious, don;t want anything specific ...
> Johan
>
^ permalink raw reply
* Re: [PATCH] Add hfp-api.txt to EXTRA_DIST
From: Marcel Holtmann @ 2010-08-11 18:55 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-bluetooth
In-Reply-To: <1281552414-2986-1-git-send-email-u.kleine-koenig@pengutronix.de>
Hi Uwe,
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Makefile.am | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
patch has been applied, but in the future stop using signed-off-by
lines. We are not using them. That is more for kernel patches.
Regards
Marcel
^ permalink raw reply
* [PATCH] Add hfp-api.txt to EXTRA_DIST
From: Uwe Kleine-König @ 2010-08-11 18:46 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1281548304.12579.243.camel@localhost.localdomain>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Makefile.am | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index aa95ed7..83a90c9 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -332,7 +332,8 @@ EXTRA_DIST += doc/manager-api.txt \
doc/adapter-api.txt doc/device-api.txt \
doc/service-api.txt doc/agent-api.txt doc/attribute-api.txt \
doc/serial-api.txt doc/network-api.txt \
- doc/input-api.txt doc/audio-api.txt doc/control-api.txt
+ doc/input-api.txt doc/audio-api.txt doc/control-api.txt \
+ doc/hfp-api.txt
AM_YFLAGS = -d
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] add mailing list as bug report address to configure.ac
From: Marcel Holtmann @ 2010-08-11 17:39 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-bluetooth
In-Reply-To: <1281104400-9019-1-git-send-email-u.kleine-koenig@pengutronix.de>
Hi Uwe,
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> configure.ac | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index b004f6e..2804d29 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -1,5 +1,5 @@
> AC_PREREQ(2.60)
> -AC_INIT(bluez, 4.69)
> +AC_INIT(bluez, 4.69, [linux-bluetooth@vger.kernel.org])
>
> AM_INIT_AUTOMAKE([foreign subdir-objects])
> AM_CONFIG_HEADER(config.h)
I prefer not doing this. Can we not just use the README file for this.
Hold on, we are already doing this ;)
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 2/2] add three missing api documents to EXTRA_DIST
From: Marcel Holtmann @ 2010-08-11 17:38 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-bluetooth
In-Reply-To: <1281104283-8792-2-git-send-email-u.kleine-koenig@pengutronix.de>
Hi Uwe,
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> there are still a few files in bluez.git/doc that are not listed in EXTRA_DIST:
>
> - bluez-docs.xml
> - gtk-doc.make
> - version.xml.in
they are not suppose to be there. They are leftovers and might be should
removed since we don't really do gtk-doc generation anymore.
> I didn't know if I should add these, too, so I just point that out.
>
> Best regards
> Uwe
>
> Makefile.am | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 4bb82fe..7007026 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -316,9 +316,12 @@ EXTRA_DIST += \
> doc/audio-api.txt \
> doc/control-api.txt \
> doc/device-api.txt \
> + doc/health-api.txt \
> + doc/hfp-api.txt \
> doc/input-api.txt \
> doc/manager-api.txt \
> doc/network-api.txt \
> + doc/node-api.txt \
> doc/serial-api.txt \
> doc/service-api.txt \
please send a patch that just adds hfp-api.txt to EXTRA_DIST.
The node-api.txt has never been implemented and might become obsolete
soon. So that should not be in the distribution.
The health-api.txt will come soon, but until we merged the code for it,
I prefer not adding it into the distribution.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/2] list api docs alphabetically and one per line in EXTRA_DIST
From: Marcel Holtmann @ 2010-08-11 17:35 UTC (permalink / raw)
To: Uwe Kleine-König; +Cc: linux-bluetooth
In-Reply-To: <1281104283-8792-1-git-send-email-u.kleine-koenig@pengutronix.de>
Hi Uwe,
> This makes further changes to the list easier to review
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Makefile.am | 17 ++++++++++++-----
> 1 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/Makefile.am b/Makefile.am
> index 65d4312..4bb82fe 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -309,11 +309,18 @@ udevdir = $(libexecdir)/udev
> dist_udev_SCRIPTS = scripts/bluetooth_serial
> endif
>
> -EXTRA_DIST += doc/manager-api.txt \
> - doc/adapter-api.txt doc/device-api.txt \
> - doc/service-api.txt doc/agent-api.txt doc/attribute-api.txt \
> - doc/serial-api.txt doc/network-api.txt \
> - doc/input-api.txt doc/audio-api.txt doc/control-api.txt
> +EXTRA_DIST += \
> + doc/adapter-api.txt \
> + doc/agent-api.txt \
> + doc/attribute-api.txt \
> + doc/audio-api.txt \
> + doc/control-api.txt \
> + doc/device-api.txt \
> + doc/input-api.txt \
> + doc/manager-api.txt \
> + doc/network-api.txt \
> + doc/serial-api.txt \
> + doc/service-api.txt \
I prefer actually to not to do this and keep them in a bit more logical
order. The manager is the initial one, followed by adapter etc.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/1] Bluetooth: RPN negotiation fix
From: Marcel Holtmann @ 2010-08-11 17:31 UTC (permalink / raw)
To: Yuri Kululin; +Cc: linux-bluetooth
In-Reply-To: <6a577b5627b8388489a21fd49f5c9d2b85a2bbef.1279866739.git.ext-yuri.kululin@nokia.com>
Hi Yuri,
> According to the ETSI 3GPP TS 07.10 the default bit rate value for RFCOMM
> is 9600 bit/s. Return this bit rate in case of RPN request and accept other
> sane bit rates proposed by the sender in RPM command.
>
> Signed-off-by: Yuri Kululin <ext-yuri.kululin@nokia.com>
> ---
> net/bluetooth/rfcomm/core.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
patch has been applied. Thanks.
Regards
Marcel
^ permalink raw reply
* Re: Support for HCI commands and events related to AMP functionality
From: ingas @ 2010-08-11 16:26 UTC (permalink / raw)
To: linux-bluetooth; +Cc: rshaffer, johan.hedberg, marcel
In-Reply-To: <1281485576-32715-1-git-send-email-ingas@codeaurora.org>
Just a clarification
> Added new HCI commands and events constructs for AMP related development.
> Will be used in updated testing tools for AMP.
>
>
> --
> Inga Stotland
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
> --
> 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
>
These changes are needed first in hcidump tool (patches will come next) to
enable parsing of AMP commands. Even though these constructs are not going
to be used elsewhere in bluez git tree (at least initially), I felt that
for the sake of consistency they should live in hci.h
The subsequent updates to hcidump will depend on this patch set in bluez.
Inga
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply
* Re: [PATCH] Fix not unmarking temporary flag when a new key is created
From: Johan Hedberg @ 2010-08-11 13:58 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <1281534567-26666-1-git-send-email-luiz.dentz@gmail.com>
Hi Luiz,
On Wed, Aug 11, 2010, Luiz Augusto von Dentz wrote:
> Device may have been marked as temporary so when a new key is generated
> that meets the requirements to be stored it should unmark temporary flag
> so the device is not removed when disconnected.
> ---
> src/dbus-hci.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/src/dbus-hci.c b/src/dbus-hci.c
> index 22f378d..8c1e10c 100644
> --- a/src/dbus-hci.c
> +++ b/src/dbus-hci.c
> @@ -746,8 +746,7 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
> else if (!bonding && old_key_type == 0xff)
> hcid_dbus_bonding_process_complete(local, peer, 0);
>
> - if (temporary)
> - device_set_temporary(device, TRUE);
> + device_set_temporary(device, temporary);
>
> return 0;
> }
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* [PATCH] Fix not unmarking temporary flag when a new key is created
From: Luiz Augusto von Dentz @ 2010-08-11 13:49 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Device may have been marked as temporary so when a new key is generated
that meets the requirements to be stored it should unmark temporary flag
so the device is not removed when disconnected.
---
src/dbus-hci.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/dbus-hci.c b/src/dbus-hci.c
index 22f378d..8c1e10c 100644
--- a/src/dbus-hci.c
+++ b/src/dbus-hci.c
@@ -746,8 +746,7 @@ int hcid_dbus_link_key_notify(bdaddr_t *local, bdaddr_t *peer,
else if (!bonding && old_key_type == 0xff)
hcid_dbus_bonding_process_complete(local, peer, 0);
- if (temporary)
- device_set_temporary(device, TRUE);
+ device_set_temporary(device, temporary);
return 0;
}
--
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