* Re: [PATCH] IrMC sync server support
From: Luiz Augusto von Dentz @ 2010-08-12 14:57 UTC (permalink / raw)
To: Marcel J.E. Mol; +Cc: linux-bluetooth
In-Reply-To: <20100812121212.GA16738@joshua.mesa.nl>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset*=utf-8''%E2%80%9Cus-ascii, Size: 1210 bytes --]
Hi Marcel,
On Thu, Aug 12, 2010 at 3:12 PM, Marcel J.E. Mol <marcel@mesa.nl> wrote:
> abstracting it made de irmc_open() more clean (e.g. no buffer fills in
> irmc_open()) and more readable in my opinion.
> The specs for devinfo.txt contain many more attributes but the currrent
> set seems to be enough. In future this may need to be extended and
> supporting scripts may be needed for getting the info. The current
> values are actually not correct I think as the will be the same for any
> device now... (hence the hint for using x-obex/capabilities in
> irmc_connect().)
It doesn't look like we can use x-obex/capabilities for this
devinfo.txt, afaik they are not the same thing, thats why I suggested
open it as any regular file so the platforms has some way to customize
it. Executing scripts is always tricky since you have to depend on
async io to suspend the request while the script is running and resume
when finished. Of course if there is no file called devinfo.txt then
we can think about a default value, what does the spec says in this
regard, is devinfo.txt mandatory? If not then we can just fail as we
do for x-obex/capabilities when not set.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: [PATCH] IrMC sync server support
From: Marcel J.E. Mol @ 2010-08-12 14:56 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281613299.12579.266.camel@localhost.localdomain>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset*=utf-8''%E2%80%9Cus-ascii, Size: 18961 bytes --]
On Thu, Aug 12, 2010 at 07:41:39AM -0400, Marcel Holtmann wrote:
> 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.
Just to have a symbolic name for the max length of the char strings in
the irmc_session struct.
All strings may need a different max length, but for now a single value
suffices. (see also the comment in irmc_connect())
>
> > +
> > +#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.
Fine
>
> > + 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.
Ok will do.
> > + 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.
Don't know :). Will change 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.
thanks, was not aware of this.
>
> > +
> > + 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.
Ok wil look into that. This one may need some more thinking to make in nice and clean
>
> > + 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.
Well, an empty buffer it is. irmc->buffer may not be NULL after as that
will be seen as 'no data available yet'.
>
> > +}
> > +
> > +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.
Oops, should have seen this when it was fixsed for pbap.
>
> > +
> > +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 ;)
Agree :)
> >
> > @@ -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
>
thanks for your comments Marcel
-Marcel
--
======-------- Marcel J.E. Mol MESA Consulting B.V.
=======--------- ph. +31-(0)6-54724868 P.O. Box 112
=======--------- marcel@mesa.nl 2630 AC Nootdorp
__==== www.mesa.nl ---____U_n_i_x______I_n_t_e_r_n_e_t____ The Netherlands ____
They couldn't think of a number, Linux user 1148 -- counter.li.org
so they gave me a name! -- Rupert Hine -- www.ruperthine.com
^ permalink raw reply
* Re: [PATCH] IrMC sync server support
From: Marcel J.E. Mol @ 2010-08-12 14:56 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTikoUKpDtT5CU7hP+QZBkbc9-p_YymXGPUbJVB+s@mail.gmail.com>
On Thu, Aug 12, 2010 at 02:16:30PM +0300, Luiz Augusto von Dentz wrote:
> 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 /*,
Fine. will do.
> 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?
abstracting it made de irmc_open() more clean (e.g. no buffer fills in
irmc_open()) and more readable in my opinion.
The specs for devinfo.txt contain many more attributes but the currrent
set seems to be enough. In future this may need to be extended and
supporting scripts may be needed for getting the info. The current
values are actually not correct I think as the will be the same for any
device now... (hence the hint for using x-obex/capabilities in
irmc_connect().)
^ permalink raw reply
* Re: [PATCHv1 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Andrei Emeltchenko @ 2010-08-12 13:33 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281612425.12579.253.camel@localhost.localdomain>
Hi Marcel,
On Thu, Aug 12, 2010 at 2:27 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> 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 remo=
ve
>> >> l2cap_conn structure. If krfcommd is in process of sending of RFCOMM =
reply
>> >> (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens.
>> >>
>> >> ...
>> >> [ =A0694.175933] Unable to handle kernel NULL pointer dereference at =
virtual 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/firmw=
are/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]) =
from [<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@padova=
n.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 y=
ou
>> > 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:
>>
>> + =A0 =A0 =A0 /* don't delete l2cap channel if sk is owned by user */
>> + =A0 =A0 =A0 if (sock_owned_by_user(sk)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 sk->sk_state =3D BT_DISCONN;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_sock_clear_timer(sk);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_sock_set_timer(sk, HZ);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 bh_unlock_sock(sk);
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
>> + =A0 =A0 =A0 }
>>
>> > 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.
>
> 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.
I am sending other version of the patch with l2cap timer which clears
L2CAP channels.
This way we are sure that channel will be deleted despite the
technique does not look
perfect...
BTW: The crash is easy to reproduce on ARM with our test tools :-)
Regards,
Andrei
^ permalink raw reply
* [PATCHv2 2/2] Bluetooth: timer check sk is not owned before freeing
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1281619537-30096-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
In timer context we might delete l2cap channel used by krfcommd.
The check makes sure that sk is not owned.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 32 ++++++++++++++++++++------------
1 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 0221d05..2f8ac5d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -73,6 +73,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
u8 code, u8 ident, u16 dlen, void *data);
/* ---- L2CAP timers ---- */
+static void l2cap_sock_set_timer(struct sock *sk, long timeout)
+{
+ BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
+ sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
+}
+
+static void l2cap_sock_clear_timer(struct sock *sk)
+{
+ BT_DBG("sock %p state %d", sk, sk->sk_state);
+ sk_stop_timer(sk, &sk->sk_timer);
+}
+
static void l2cap_sock_timeout(unsigned long arg)
{
struct sock *sk = (struct sock *) arg;
@@ -82,6 +94,14 @@ static void l2cap_sock_timeout(unsigned long arg)
bh_lock_sock(sk);
+ if (sock_owned_by_user(sk)) {
+ /* sk is owned by user. Try again later */
+ l2cap_sock_set_timer(sk, HZ * 2);
+ bh_unlock_sock(sk);
+ sock_put(sk);
+ return;
+ }
+
if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONFIG)
reason = ECONNREFUSED;
else if (sk->sk_state == BT_CONNECT &&
@@ -98,18 +118,6 @@ static void l2cap_sock_timeout(unsigned long arg)
sock_put(sk);
}
-static void l2cap_sock_set_timer(struct sock *sk, long timeout)
-{
- BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout);
- sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout);
-}
-
-static void l2cap_sock_clear_timer(struct sock *sk)
-{
- BT_DBG("sock %p state %d", sk, sk->sk_state);
- sk_stop_timer(sk, &sk->sk_timer);
-}
-
/* ---- L2CAP channels ---- */
static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid)
{
--
1.7.0.4
^ permalink raw reply related
* [PATCHv2 1/2] Bluetooth: Check sk is not owned before freeing l2cap_conn
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1281619537-30096-1-git-send-email-Andrei.Emeltchenko.news@gmail.com>
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
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>
Fixes: NB#164721 - Device reboots while executing OBEX FTP tests
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap.c | 25 +++++++++++++++++++++++++
1 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 1874ece..0221d05 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -2708,6 +2708,13 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd
break;
default:
+ /* 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);
+ break;
+ }
l2cap_chan_del(sk, ECONNREFUSED);
break;
}
@@ -2914,6 +2921,15 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd
del_timer(&l2cap_pi(sk)->monitor_timer);
}
+ /* 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;
+ }
+
l2cap_chan_del(sk, ECONNRESET);
bh_unlock_sock(sk);
@@ -2944,6 +2960,15 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd
del_timer(&l2cap_pi(sk)->monitor_timer);
}
+ /* 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;
+ }
+
l2cap_chan_del(sk, 0);
bh_unlock_sock(sk);
--
1.7.0.4
^ permalink raw reply related
* [PATCHv2 0/2] Fix kernel crash in rfcomm/l2cap
From: Emeltchenko Andrei @ 2010-08-12 13:25 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Version 2 of patches fixing kernel crash in RFCOMM / L2CAP.
Do not delete l2cap channel and socket sk when sk is owned by user.
To delete l2cap channel standard timer is used.
lock_sock and release_sock do not hold a normal spinlock directly but
instead hold the owner field. This means bh_lock_sock can still execute
even if the socket is "locked". More info can be found here:
http://www.linuxfoundation.org/collaborate/workgroups/networking/socketlocks
Andrei Emeltchenko (2):
Bluetooth: Check sk is not owned before freeing l2cap_conn
Bluetooth: timer check sk is not owned before freeing
net/bluetooth/l2cap.c | 57 ++++++++++++++++++++++++++++++++++++++----------
1 files changed, 45 insertions(+), 12 deletions(-)
^ permalink raw reply
* Question about phonebook_add_contact
From: Radoslaw Jablonski @ 2010-08-12 13:13 UTC (permalink / raw)
To: linux-bluetooth@vger.kernel.org
Hi!
I've got question about this part of code from beginning
phonebook_add_contact func:
if ((contact->numbers == NULL && (contact->fullname == NULL ||
contact->fullname[0] == '\0')))
return;
Is this checking really needed in that place? It is possible to have
contacts data
in phonebook that have all data except phone set (by example: email
addresses,
main address etc). In that condition when we want to pull target vcard
entry, then
empty string will be returned even if user send ATTRIBUTE_MASK to
include all fields
in the result.
I'm asking because I have problem with "0.vcf" vcard (this is
responsible for personal
information of device owner). This entry by default does not have any
phone's data
filled in address book but it is mandatory to put this in vcard-listing.
Actually, I am forced to initialize contact->numbers list with empty
number even if
none of number is available to return correct result for "0.vcf."(in
phonebook-tracker)...
Can this 'contact->numbers' checking be removed from there or maybe
other phonebooks
rely on this behaviour?
BR,
Radek
^ permalink raw reply
* [PATCH] Changes in HDP API.
From: Jose Antonio Santos Cadenas @ 2010-08-12 13:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
Now the connection is made directly to a device and bluetoothd daemon
guess the PSM connection and the MDEPID based on the data get from the
SDP record.
Also a property that indicates the MainChannel is added to the
HealthDevice object.
---
doc/health-api.txt | 119 +++++++++++++++++++++++-----------------------------
1 files changed, 53 insertions(+), 66 deletions(-)
diff --git a/doc/health-api.txt b/doc/health-api.txt
index f469df3..0a7ae73 100644
--- a/doc/health-api.txt
+++ b/doc/health-api.txt
@@ -14,12 +14,9 @@ Object path /org/bluez/
Methods:
- object CreateApplication(dict config, object agent)
+ object CreateApplication(dict config)
- Returns the path of the new registered application. The agent
- parameter is the path of the object with the callbacks to
- notify events (see org.bluez.HealthAgent at the end
- of this document).
+ Returns the path of the new registered application.
Dict is defined as bellow:
{
@@ -39,32 +36,33 @@ Methods:
Closes the HDP application identified by the object path. Also
application will be closed if the process that started it leaves
- the bus.
+ the bus. Only the creator of the application will be able to
+ destroy it.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.NotFound
+ org.bluez.Error.NotAllowed
--------------------------------------------------------------------------------
Service org.bluez
-Interface org.bluez.HealthApplication
-Object path [variable prefix]/health_app_ZZZZ
+Interface org.bluez.HealthDevice
+Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX
Methods:
- void Echo(object service)
+ Boolean Echo()
Sends an echo petition to the remote service. Returns True if
response matches with the buffer sent. If some error is detected
- False value is returned and the associated MCL is closed.
+ False value is returned.
Possible errors: org.bluez.Error.InvalidArguments
org.bluez.Error.OutOfRange
- object CreateChannel(object service, string type)
+ object CreateChannel(object application, string configuration)
- Creates a new data channel with the indicated config to the
- remote Service.
+ Creates a new data channel.
The configuration should indicate the channel quality of
service using one of this values "Reliable", "Streaming", "Any".
@@ -76,16 +74,39 @@ Methods:
void DestroyChannel(object channel)
- Destroys the data channel object.
+ Destroys the data channel object. Only the creator of the
+ channel or the creator of the HealtApplication that received the
+ data channel will be able to destroy it
Possible errors: org.bluez.Error.InvalidArguments
- orb.bluez.Error.NotFound
+ org.bluez.Error.NotFound
+ org.bluez.Error.NotAllowed
---------------------------------------------------------------------------------
+Signals:
-Service org.bluez
-Interface org.bluez.HealthService
-Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/hdp_YYYY
+ void ChannelConnected(object channel)
+
+ This signal is launched when a new data channel is created or
+ when a known data channel is reconnected.
+
+ void ChannelDeleted(object channel)
+
+ This signal is launched when a data channel is deleted.
+
+ After this signal the data channel path will not be valid and
+ its path can be reused for future data channels.
+
+ void PropertyChanged(string name, variant value)
+
+ This signal indicates a changed value of the given property.
+
+Properties:
+
+ object MainChannel [readonly]
+
+ The first reliable channel opened. It is needed by upper
+ applications in order to send specific protocol data units. The
+ first reliable can change after a reconnection.
--------------------------------------------------------------------------------
@@ -94,6 +115,9 @@ Interface org.bluez.HealthChannel
Object path [variable prefix]/{hci0,hci1,...}/dev_XX_XX_XX_XX_XX_XX/
hdp_YYYY/channel_ZZ
+Only the process that created the data channel or the creator of the
+HealthApplication that received it will be able to call this methods.
+
Methods:
dict GetProperties()
@@ -101,68 +125,31 @@ Methods:
Returns all properties for the interface. See the properties
section for available properties.
+ Posible errors: org.bluez.Error.NotAllowed
+
fd Acquire()
Returns the file descriptor for this data channel. If the data
channel is not connected it will also reconnect.
Possible errors: org.bluez.Error.NotConnected
+ org.bluez.Error.NotAllowed
void Release()
Releases the fd. Application should also need to close() it.
+ Possible errors: org.bluez.Error.NotAcquired
+ org.bluez.Error.NotAllowed
+
Properties:
- string Type (read only)
+ string Type [readonly]
The quality of service of the data channel. ("Reliable" or
"Streaming")
- object Service (read only)
-
- Identifies the Remote Service that is connected with. Maps with
- a HealthService object.
-
-HealthAgent hierarchy
-=====================
-
-(this object is implemented by the HDP user in order to receive notifications)
-
-Service unique name
-Interface org.bluez.HealthAgent
-Object path freely definable
-
-Methods:
-
- void Release()
-
- This method gets called when the service daemon unregisters the
- agent. An agent can use it to do cleanup tasks. There is no need
- to unregister the agent, because when this method gets called it
- has already been unregistered.
-
- void ServiceDiscovered(object service)
-
- This method is called when a device containing an HDP
- application is paired or when the method Update of the
- HealthManager is called and new HealthServices are discovered.
- The method will be called once for each HealthService.
-
- void ServiceRemoved(object service)
-
- This is called if during an Update some HealthServices
- have disappeared. The method is called once for each removed
- HealthService.
-
- void ChannelConnected(object channel)
-
- This method is called when a new data channel is created or when
- a known data channel is reconnected.
-
- void ChannelDeleted(object channel)
-
- This method is called when a data channel is deleted.
+ object Device [readonly]
- After this call the data channel path will not be valid and can
- be reused for future creation of data channels.
+ Identifies the Remote Device that is connected with. Maps with
+ a HealthDevice object.
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCHv2] Bluetooth: new sockopt to enter active state when sending data
From: Marcel Holtmann @ 2010-08-12 13:07 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinp-wXzgUU3dSNteSGdBmgtcB8S+sWZZ-9b0BnD@mail.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.
>
> This can be done.
>
> > 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.
>
> I felt that Sniff Mode is a property of ACL link and not L2CAP or RFCOMM socket.
>
> There is already code in the same functions which use the same
> technique to access
> following hci_conn fields:
>
> l2cap_pi(sk)->conn->hcon->handle
> l2cap_pi(sk)->conn->hcon->dev_class
that is for pure read-only access. If multiple L2CAP sockets are open
you have to be smart about the policy you put onto your ACL link.
Regards
Marcel
^ permalink raw reply
* Re: [PATCHv2] Bluetooth: new sockopt to enter active state when sending data
From: Andrei Emeltchenko @ 2010-08-12 12:25 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1281612715.12579.257.camel@localhost.localdomain>
Hi Marcel,
On Thu, Aug 12, 2010 at 2:31 PM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> 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>
>> ---
>> =A0include/net/bluetooth/hci_core.h | =A0 =A01 +
>> =A0include/net/bluetooth/l2cap.h =A0 =A0| =A0 =A02 ++
>> =A0net/bluetooth/hci_conn.c =A0 =A0 =A0 =A0 | =A0 =A07 ++++++-
>> =A0net/bluetooth/l2cap.c =A0 =A0 =A0 =A0 =A0 =A0| =A0 15 +++++++++++++++
>> =A04 files changed, 24 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hc=
i_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 {
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 auth_type;
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 sec_level;
>> =A0 =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 power_save;
>> + =A0 =A0 __u8 =A0 =A0 =A0 =A0 =A0 =A0 force_active;
>> =A0 =A0 =A0 __u16 =A0 =A0 =A0 =A0 =A0 =A0disc_timeout;
>> =A0 =A0 =A0 unsigned long =A0 =A0pend;
>>
>> 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 {
>> =A0#define L2CAP_LM_RELIABLE =A0 =A00x0010
>> =A0#define L2CAP_LM_SECURE =A0 =A0 =A0 =A0 =A0 =A0 =A00x0020
>>
>> +#define L2CAP_FORCE_ACTIVE_MODE =A0 =A0 =A00x04
>> +
>> =A0/* L2CAP command codes */
>> =A0#define L2CAP_COMMAND_REJ =A0 =A00x01
>> =A0#define L2CAP_CONN_REQ =A0 =A0 =A0 =A0 =A0 =A0 =A0 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)
>> =A0 =A0 =A0 conn->auth_type =3D HCI_AT_GENERAL_BONDING;
>>
>> =A0 =A0 =A0 conn->power_save =3D 1;
>> + =A0 =A0 /* Do not enter active state by default */
>> + =A0 =A0 conn->force_active =3D 0;
>> =A0 =A0 =A0 conn->disc_timeout =3D HCI_DISCONN_TIMEOUT;
>>
>> =A0 =A0 =A0 switch (type) {
>> @@ -505,7 +507,10 @@ void hci_conn_enter_active_mode(struct hci_conn *co=
nn)
>> =A0 =A0 =A0 if (test_bit(HCI_RAW, &hdev->flags))
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>>
>> - =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF || !conn->power_save)
>> + =A0 =A0 if (conn->mode !=3D HCI_CM_SNIFF)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto timer;
>> +
>> + =A0 =A0 if (!conn->power_save && !conn->force_active)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto timer;
>>
>> =A0 =A0 =A0 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 socke=
t *sock, int optname, char __us
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->force_reliable =3D (opt & L2CA=
P_LM_RELIABLE);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> + =A0 =A0 case L2CAP_FORCE_ACTIVE_MODE:
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (get_user(opt, (u32 __user *) optval)) {
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EFAULT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> + =A0 =A0 =A0 =A0 =A0 =A0 }
>> +
>> + =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(sk)->conn->hcon->force_active =3D opt=
;
>> + =A0 =A0 =A0 =A0 =A0 =A0 break;
>> +
>> =A0 =A0 =A0 default:
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOPROTOOPT;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>> @@ -2058,6 +2067,12 @@ static int l2cap_sock_getsockopt_old(struct socke=
t *sock, int optname, char __us
>>
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>>
>> + =A0 =A0 case L2CAP_FORCE_ACTIVE_MODE:
>> + =A0 =A0 =A0 =A0 =A0 =A0 if (put_user(l2cap_pi(sk)->conn->hcon->force_a=
ctive,
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 (u32 __user *) optval))
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -EFAULT;
>> + =A0 =A0 =A0 =A0 =A0 =A0 break;
>> +
>> =A0 =A0 =A0 default:
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 err =3D -ENOPROTOOPT;
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 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.
This can be done.
> 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.
I felt that Sniff Mode is a property of ACL link and not L2CAP or RFCOMM so=
cket.
There is already code in the same functions which use the same
technique to access
following hci_conn fields:
l2cap_pi(sk)->conn->hcon->handle
l2cap_pi(sk)->conn->hcon->dev_class
Regards,
Andrei
^ permalink raw reply
* Re: resending patch to retry reset in case it failed
From: Oliver Neukum @ 2010-08-12 12:16 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth, linux-usb
In-Reply-To: <1281612323.12579.252.camel@localhost.localdomain>
Am Donnerstag, 12. August 2010, 13:25:23 schrieb Marcel Holtmann:
> 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.
I am very open to alternatives. But without that it is needed to make the
hardware work.
> Tell me which device is causing this and I might be able to reproduce
> this and find small less ugly fix.
I happens with this hardware from Broadcom. There may be others I don't
know about. It is an internal dongle. I don't know whether it can be found in
external hardware.
Regards
Oliver
Bus 002 Device 002: ID 0a5c:21b4 Broadcom Corp.
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 224 Wireless
bDeviceSubClass 1 Radio Frequency
bDeviceProtocol 1 Bluetooth
bMaxPacketSize0 64
idVendor 0x0a5c Broadcom Corp.
idProduct 0x21b4
bcdDevice 4.81
iManufacturer 1
iProduct 2
iSerial 3
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 216
bNumInterfaces 4
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xe0
Self Powered
Remote Wakeup
MaxPower 0mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 3
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0010 1x 16 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0000 1x 0 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 1
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0009 1x 9 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 2
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0011 1x 17 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 3
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 4
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 5
bNumEndpoints 2
bInterfaceClass 224 Wireless
bInterfaceSubClass 1 Radio Frequency
bInterfaceProtocol 1 Bluetooth
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x03 EP 3 OUT
bmAttributes 1
Transfer Type Isochronous
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 2
bAlternateSetting 0
bNumEndpoints 2
bInterfaceClass 255 Vendor Specific Class
bInterfaceSubClass 255 Vendor Specific Subclass
bInterfaceProtocol 255 Vendor Specific Protocol
iInterface 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x84 EP 4 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 1
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x04 EP 4 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0020 1x 32 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 3
bAlternateSetting 0
bNumEndpoints 0
bInterfaceClass 254 Application Specific Interface
bInterfaceSubClass 1 Device Firmware Update
bInterfaceProtocol 1
iInterface 0
** UNRECOGNIZED: 07 21 07 88 13 40 00
^ permalink raw reply
* Re: why is hciops a plugin ?
From: Marcel Holtmann @ 2010-08-12 11:43 UTC (permalink / raw)
To: Johan Hedberg; +Cc: Pavan Savoy, linux-bluetooth@vger.kernel.org
In-Reply-To: <20100812030707.GA17784@jh-x301>
Hi Johan,
> > 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).
this is correct. We will not require userspace to do HCI event
processing and filtering anymore. It will be a dedicated side channel.
Regards
Marcel
^ permalink raw reply
* 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
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