Forward error codes from tty core to the rfcomm_init caller instead of using
generic -1 errors.
Signed-off-by: David Herrmann <[email protected]>
---
net/bluetooth/rfcomm/tty.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index c258796..2b753a3 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = {
int __init rfcomm_init_ttys(void)
{
+ int error;
+
rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS);
if (!rfcomm_tty_driver)
- return -1;
+ return -ENOMEM;
rfcomm_tty_driver->owner = THIS_MODULE;
rfcomm_tty_driver->driver_name = "rfcomm";
@@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void)
rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
- if (tty_register_driver(rfcomm_tty_driver)) {
+ error = tty_register_driver(rfcomm_tty_driver);
+ if (error) {
BT_ERR("Can't register RFCOMM TTY driver");
put_tty_driver(rfcomm_tty_driver);
- return -1;
+ return error;
}
BT_INFO("RFCOMM TTY layer initialized");
--
1.7.7
Hi David,
* David Herrmann <[email protected]> [2011-10-24 15:30:58 +0200]:
> Remove old tasklets and replace by workqueue. To avoid reentrancy (which
> tasklets always avoid) we use the system_nrt_wq.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> net/bluetooth/rfcomm/tty.c | 16 +++++++++-------
> 1 files changed, 9 insertions(+), 7 deletions(-)
Both patches have been applied, thanks.
Gustavo
On Mon, Oct 24, 2011 at 3:57 PM, Anderson Briglia
<[email protected]> wrote:
> Hi David,
>
> On Mon, Oct 24, 2011 at 3:30 PM, David Herrmann
> <[email protected]> wrote:
>> Forward error codes from tty core to the rfcomm_init caller instead of using
>> generic -1 errors.
>>
>> Signed-off-by: David Herrmann <[email protected]>
>> ---
>> ?net/bluetooth/rfcomm/tty.c | ? ?9 ++++++---
>> ?1 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
>> index c258796..2b753a3 100644
>> --- a/net/bluetooth/rfcomm/tty.c
>> +++ b/net/bluetooth/rfcomm/tty.c
>> @@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = {
>>
>> ?int __init rfcomm_init_ttys(void)
>> ?{
>> + ? ? ? int error;
>> +
>> ? ? ? ?rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS);
>> ? ? ? ?if (!rfcomm_tty_driver)
>> - ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? ? return -ENOMEM;
>>
>> ? ? ? ?rfcomm_tty_driver->owner ? ? ? ?= THIS_MODULE;
>> ? ? ? ?rfcomm_tty_driver->driver_name ?= "rfcomm";
>> @@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void)
>> ? ? ? ?rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
>> ? ? ? ?tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
>>
>> - ? ? ? if (tty_register_driver(rfcomm_tty_driver)) {
>> + ? ? ? error = tty_register_driver(rfcomm_tty_driver);
>> + ? ? ? if (error) {
>> ? ? ? ? ? ? ? ?BT_ERR("Can't register RFCOMM TTY driver");
>> ? ? ? ? ? ? ? ?put_tty_driver(rfcomm_tty_driver);
>> - ? ? ? ? ? ? ? return -1;
>> + ? ? ? ? ? ? ? return error;
>
> Since you are defining a new variable (error), how about use it on
> other error paths and add a "goto out" and have just one return path?
Isn't it quite common style to have the first error path return
directly? And then the other error paths use goto. However, there is
only one additional error path left so I don't see any way to make
this look nicer.
With more than 2 error paths I agree, a goto would be more readable,
but I cannot see this making this function look nicer. Anyway, if you
guys want, I can resend with goto's.
> Regards,
>
> Anderson Briglia
Regards
David
Hi David,
On Mon, Oct 24, 2011 at 3:30 PM, David Herrmann
<[email protected]> wrote:
> Forward error codes from tty core to the rfcomm_init caller instead of using
> generic -1 errors.
>
> Signed-off-by: David Herrmann <[email protected]>
> ---
> ?net/bluetooth/rfcomm/tty.c | ? ?9 ++++++---
> ?1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
> index c258796..2b753a3 100644
> --- a/net/bluetooth/rfcomm/tty.c
> +++ b/net/bluetooth/rfcomm/tty.c
> @@ -1155,9 +1155,11 @@ static const struct tty_operations rfcomm_ops = {
>
> ?int __init rfcomm_init_ttys(void)
> ?{
> + ? ? ? int error;
> +
> ? ? ? ?rfcomm_tty_driver = alloc_tty_driver(RFCOMM_TTY_PORTS);
> ? ? ? ?if (!rfcomm_tty_driver)
> - ? ? ? ? ? ? ? return -1;
> + ? ? ? ? ? ? ? return -ENOMEM;
>
> ? ? ? ?rfcomm_tty_driver->owner ? ? ? ?= THIS_MODULE;
> ? ? ? ?rfcomm_tty_driver->driver_name ?= "rfcomm";
> @@ -1172,10 +1174,11 @@ int __init rfcomm_init_ttys(void)
> ? ? ? ?rfcomm_tty_driver->init_termios.c_lflag &= ~ICANON;
> ? ? ? ?tty_set_operations(rfcomm_tty_driver, &rfcomm_ops);
>
> - ? ? ? if (tty_register_driver(rfcomm_tty_driver)) {
> + ? ? ? error = tty_register_driver(rfcomm_tty_driver);
> + ? ? ? if (error) {
> ? ? ? ? ? ? ? ?BT_ERR("Can't register RFCOMM TTY driver");
> ? ? ? ? ? ? ? ?put_tty_driver(rfcomm_tty_driver);
> - ? ? ? ? ? ? ? return -1;
> + ? ? ? ? ? ? ? return error;
Since you are defining a new variable (error), how about use it on
other error paths and add a "goto out" and have just one return path?
Regards,
Anderson Briglia
> ? ? ? ?}
>
> ? ? ? ?BT_INFO("RFCOMM TTY layer initialized");
> --
> 1.7.7
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
INdT - Instituto Nokia de tecnologia
+55 92 2126 1122
+55 92 8423 3183
Remove old tasklets and replace by workqueue. To avoid reentrancy (which
tasklets always avoid) we use the system_nrt_wq.
Signed-off-by: David Herrmann <[email protected]>
---
net/bluetooth/rfcomm/tty.c | 16 +++++++++-------
1 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index 2b753a3..947f1b3 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -34,6 +34,7 @@
#include <linux/capability.h>
#include <linux/slab.h>
#include <linux/skbuff.h>
+#include <linux/workqueue.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -65,7 +66,7 @@ struct rfcomm_dev {
struct rfcomm_dlc *dlc;
struct tty_struct *tty;
wait_queue_head_t wait;
- struct tasklet_struct wakeup_task;
+ struct work_struct wakeup_task;
struct device *tty_dev;
@@ -81,7 +82,7 @@ static void rfcomm_dev_data_ready(struct rfcomm_dlc *dlc, struct sk_buff *skb);
static void rfcomm_dev_state_change(struct rfcomm_dlc *dlc, int err);
static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig);
-static void rfcomm_tty_wakeup(unsigned long arg);
+static void rfcomm_tty_wakeup(struct work_struct *work);
/* ---- Device functions ---- */
static void rfcomm_dev_destruct(struct rfcomm_dev *dev)
@@ -257,7 +258,7 @@ static int rfcomm_dev_add(struct rfcomm_dev_req *req, struct rfcomm_dlc *dlc)
atomic_set(&dev->opened, 0);
init_waitqueue_head(&dev->wait);
- tasklet_init(&dev->wakeup_task, rfcomm_tty_wakeup, (unsigned long) dev);
+ INIT_WORK(&dev->wakeup_task, rfcomm_tty_wakeup);
skb_queue_head_init(&dev->pending);
@@ -351,7 +352,7 @@ static void rfcomm_wfree(struct sk_buff *skb)
struct rfcomm_dev *dev = (void *) skb->sk;
atomic_sub(skb->truesize, &dev->wmem_alloc);
if (test_bit(RFCOMM_TTY_ATTACHED, &dev->flags))
- tasklet_schedule(&dev->wakeup_task);
+ queue_work(system_nrt_wq, &dev->wakeup_task);
rfcomm_dev_put(dev);
}
@@ -635,9 +636,10 @@ static void rfcomm_dev_modem_status(struct rfcomm_dlc *dlc, u8 v24_sig)
}
/* ---- TTY functions ---- */
-static void rfcomm_tty_wakeup(unsigned long arg)
+static void rfcomm_tty_wakeup(struct work_struct *work)
{
- struct rfcomm_dev *dev = (void *) arg;
+ struct rfcomm_dev *dev = container_of(work, struct rfcomm_dev,
+ wakeup_task);
struct tty_struct *tty = dev->tty;
if (!tty)
return;
@@ -762,7 +764,7 @@ static void rfcomm_tty_close(struct tty_struct *tty, struct file *filp)
rfcomm_dlc_close(dev->dlc, 0);
clear_bit(RFCOMM_TTY_ATTACHED, &dev->flags);
- tasklet_kill(&dev->wakeup_task);
+ cancel_work_sync(&dev->wakeup_task);
rfcomm_dlc_lock(dev->dlc);
tty->driver_data = NULL;
--
1.7.7