2009-05-04 15:08:12

by alok barsode

[permalink] [raw]
Subject: [PATCH 1/3] Do not exit if adapter_ops setup fails.

From: Alok Barsode <[email protected]>

---
src/adapter.c | 15 ++++++++++-----
src/main.c | 5 +----
2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index df85f6b..7ac361b 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -406,9 +406,11 @@ static int set_mode(struct btd_adapter *adapter, uint8_t new_mode)
scan_enable = mode2scan(new_mode);

if (!adapter->up && scan_enable != SCAN_DISABLED) {
- err = adapter_ops->start(adapter->dev_id);
- if (err < 0)
- return err;
+ if (adapter_ops) {
+ err = adapter_ops->start(adapter->dev_id);
+ if (err < 0)
+ return err;
+ }
}

dd = hci_open_dev(adapter->dev_id);
@@ -3095,13 +3097,16 @@ int btd_register_adapter_ops(struct btd_adapter_ops *btd_adapter_ops)

void btd_adapter_cleanup_ops()
{
- adapter_ops->cleanup();
+ if (adapter_ops)
+ adapter_ops->cleanup();
}

int adapter_ops_setup()
{
- if (!adapter_ops)
+ if (!adapter_ops) {
+ error("Unable to initialize adapter operations");
return -EINVAL;
+ }

return adapter_ops->setup();
}
diff --git a/src/main.c b/src/main.c
index 884257f..8dd2cee 100644
--- a/src/main.c
+++ b/src/main.c
@@ -412,10 +412,7 @@ int main(int argc, char *argv[])

event_loop = g_main_loop_new(NULL, FALSE);

- if (adapter_ops_setup() < 0) {
- error("adapter_ops_setup failed");
- exit(1);
- }
+ adapter_ops_setup();

starting = FALSE;

--
1.5.6.3



2009-05-04 15:30:50

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 3/3] Adding stop() functionality to hciops plugin.

Hi Alok,

On Mon, May 04, 2009, [email protected] wrote:
> From: Alok Barsode <[email protected]>
>
> ---
> plugins/hciops.c | 24 ++++++++++++++++++++++++
> src/adapter.c | 25 ++++++++++++++-----------
> src/adapter.h | 1 +
> 3 files changed, 39 insertions(+), 11 deletions(-)

This patch is fine assuming that we don't want to exit when
adapter_ops_setup fails. Though now it's apparent that the code would get
simpler if we would exit() in that case since then we don't need to have
any "if (adapter_ops)" checks later in the code.

Johan

2009-05-04 15:27:56

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 2/3] Code cleanup in adapter.c.

Hi Alok,

This patch looks ok'ish except for the following unnecessary empty line
that it adds:

On Mon, May 04, 2009, [email protected] wrote:
> if (dev_down == FALSE)
> hci_send_cmd(dd, OGF_HOST_CTL, OCF_WRITE_SCAN_ENABLE,
> 1, &scan_mode);
>
> +
> if (adapter->initialized == FALSE) {
> load_drivers(adapter);
> load_devices(adapter);

Johan

2009-05-04 15:25:58

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 1/3] Do not exit if adapter_ops setup fails.

Hi Alok,

On Mon, May 04, 2009, [email protected] wrote:
> @@ -412,10 +412,7 @@ int main(int argc, char *argv[])
>
> event_loop = g_main_loop_new(NULL, FALSE);
>
> - if (adapter_ops_setup() < 0) {
> - error("adapter_ops_setup failed");
> - exit(1);
> - }
> + adapter_ops_setup();
>
> starting = FALSE;

Do we really want this behavior? At the very least there should be a loud
error() in the case the adapter_ops_setup fails. Marcel, any opinion?

Johan

2009-05-04 15:08:14

by alok barsode

[permalink] [raw]
Subject: [PATCH 3/3] Adding stop() functionality to hciops plugin.

From: Alok Barsode <[email protected]>

---
plugins/hciops.c | 24 ++++++++++++++++++++++++
src/adapter.c | 25 ++++++++++++++-----------
src/adapter.h | 1 +
3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/plugins/hciops.c b/plugins/hciops.c
index ba4b69d..2e91129 100644
--- a/plugins/hciops.c
+++ b/plugins/hciops.c
@@ -439,10 +439,34 @@ done:
return err;
}

+static int hciops_stop(int index)
+{
+ int dd;
+ int err = 0;
+
+ dd = hci_open_dev(index);
+ if (dd < 0)
+ return -EIO;
+
+ if (ioctl(dd, HCIDEVDOWN, index) == 0)
+ goto done; /* on success */
+
+ if (errno != EALREADY) {
+ err = -errno;
+ error("Can't stop device hci%d: %s (%d)",
+ index, strerror(errno), errno);
+ }
+
+done:
+ hci_close_dev(dd);
+ return err;
+}
+
static struct btd_adapter_ops hci_ops = {
.setup = hciops_setup,
.cleanup = hciops_cleanup,
.start = hciops_start,
+ .stop = hciops_stop,
};

static int hciops_init(void)
diff --git a/src/adapter.c b/src/adapter.c
index 2135795..007702a 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -426,11 +426,12 @@ static int set_mode(struct btd_adapter *adapter, uint8_t new_mode)
};

hci_send_req(dd, &rq, HCI_REQ_TIMEOUT);
+ hci_close_dev(dd);

- if (ioctl(dd, HCIDEVDOWN, adapter->dev_id) < 0) {
- err = -errno;
- hci_close_dev(dd);
- return err;
+ if (adapter_ops) {
+ err = adapter_ops->stop(adapter->dev_id);
+ if (err < 0)
+ return err;
}

adapter->off_requested = TRUE;
@@ -438,6 +439,10 @@ static int set_mode(struct btd_adapter *adapter, uint8_t new_mode)
goto done;
}

+ dd = hci_open_dev(adapter->dev_id);
+ if (dd < 0)
+ return -EIO;
+
if (current_scan != scan_enable) {
err = hci_send_cmd(dd, OGF_HOST_CTL, OCF_WRITE_SCAN_ENABLE,
1, &scan_enable);
@@ -2085,7 +2090,9 @@ proceed:
}

if (dev_down) {
- ioctl(dd, HCIDEVDOWN, adapter->dev_id);
+ if (adapter_ops)
+ adapter_ops->stop(adapter->dev_id);
+
adapter->off_requested = TRUE;
return 1;
} else
@@ -2469,12 +2476,8 @@ void adapter_remove(struct btd_adapter *adapter)

/* Return adapter to down state if it was not up on init */
if (adapter->up && !adapter->already_up) {
- int dd = hci_open_dev(adapter->dev_id);
- if (dd < 0)
- goto done;
-
- ioctl(dd, HCIDEVDOWN, adapter->dev_id);
- hci_close_dev(dd);
+ if (adapter_ops)
+ adapter_ops->stop(adapter->dev_id);
}

done:
diff --git a/src/adapter.h b/src/adapter.h
index 67db648..acf680b 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -153,6 +153,7 @@ struct btd_adapter_ops {
int (*setup) (void);
void (*cleanup) (void);
int (*start) (int index);
+ int (*stop) (int index);
};

int btd_register_adapter_ops(struct btd_adapter_ops *btd_adapter_ops);
--
1.5.6.3


2009-05-04 15:08:13

by alok barsode

[permalink] [raw]
Subject: [PATCH 2/3] Code cleanup in adapter.c.

From: Alok Barsode <[email protected]>

Not passing device descriptor to adater_up.
---
src/adapter.c | 17 +++++++++++++----
1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 7ac361b..2135795 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -2001,11 +2001,12 @@ static int get_pairable_timeout(const char *src)
return main_opts.pairto;
}

-static int adapter_up(struct btd_adapter *adapter, int dd)
+static int adapter_up(struct btd_adapter *adapter)
{
char mode[14], srcaddr[18];
uint8_t scan_mode;
gboolean powered, dev_down = FALSE;
+ int dd;

ba2str(&adapter->bdaddr, srcaddr);

@@ -2048,7 +2049,7 @@ static int adapter_up(struct btd_adapter *adapter, int dd)
else
write_device_mode(&adapter->bdaddr, mode);

- return adapter_up(adapter, dd);
+ return adapter_up(adapter);
} else if (!g_str_equal(mode, "connectable") &&
adapter->discov_timeout == 0) {
/* Set discoverable only if timeout is 0 */
@@ -2058,10 +2059,16 @@ static int adapter_up(struct btd_adapter *adapter, int dd)
}

proceed:
+ dd = hci_open_dev(adapter->dev_id);
+
+ if (dd < 0)
+ return -EIO;
+
if (dev_down == FALSE)
hci_send_cmd(dd, OGF_HOST_CTL, OCF_WRITE_SCAN_ENABLE,
1, &scan_mode);

+
if (adapter->initialized == FALSE) {
load_drivers(adapter);
load_devices(adapter);
@@ -2086,6 +2093,8 @@ proceed:
ADAPTER_INTERFACE, "Powered",
DBUS_TYPE_BOOLEAN, &powered);

+ hci_close_dev(dd);
+
return 0;
}

@@ -2199,10 +2208,10 @@ setup:
adapter->state &= ~STD_INQUIRY;

adapter_setup(adapter, dd);
- err = adapter_up(adapter, dd);
-
hci_close_dev(dd);

+ err = adapter_up(adapter);
+
info("Adapter %s has been enabled", adapter->path);

return err;
--
1.5.6.3