[PATCH 1/5] btmgmt: Remove unneeded code

[PATCH 1/5] btmgmt: Remove unneeded code

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 tools/btmgmt.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index 1d71d74..f31d8b1 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -555,7 +555,6 @@ static void user_confirm(uint16_t index, uint16_t len, const void *param,
 	rsp_len = strlen(rsp);
 	if (rsp[rsp_len - 1] == '\n') {
 		rsp[rsp_len - 1] = '\0';
-		rsp_len--;
 	}
 
 	if (rsp[0] == 'y' || rsp[0] == 'Y')
-- 
1.8.3.2

[PATCH 2/5] avdtp: Remove unneeded local variable

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 profiles/audio/avdtp.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index f866b39..e12ad9d 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -773,10 +773,9 @@ static int get_send_buffer_size(int sk)
 	socklen_t optlen = sizeof(size);
 
 	if (getsockopt(sk, SOL_SOCKET, SO_SNDBUF, &size, &optlen) < 0) {
-		int err = -errno;
-		error("getsockopt(SO_SNDBUF) failed: %s (%d)", strerror(-err),
-									-err);
-		return err;
+		error("getsockopt(SO_SNDBUF) failed: %s (%d)", strerror(errno),
+									errno);
+		return -errno;
 	}
 
 	/*
@@ -792,10 +791,9 @@ static int set_send_buffer_size(int sk, int size)
 	socklen_t optlen = sizeof(size);
 
 	if (setsockopt(sk, SOL_SOCKET, SO_SNDBUF, &size, optlen) < 0) {
-		int err = -errno;
-		error("setsockopt(SO_SNDBUF) failed: %s (%d)", strerror(-err),
-									-err);
-		return err;
+		error("setsockopt(SO_SNDBUF) failed: %s (%d)", strerror(errno),
+									errno);
+		return -errno;
 	}
 
 	return 0;
-- 
1.8.3.2

Re: [PATCH 2/5] avdtp: Remove unneeded local variable

[Johan Hedberg]

Hi Andrei,

On Mon, Dec 16, 2013, Andrei Emeltchenko wrote:
> ---
>  profiles/audio/avdtp.c | 14 ++++++--------
>  1 file changed, 6 insertions(+), 8 deletions(-)

I've applied all patches except these ones claiming to remove an
unneeded err variable.

> diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
> index f866b39..e12ad9d 100644
> --- a/profiles/audio/avdtp.c
> +++ b/profiles/audio/avdtp.c
> @@ -773,10 +773,9 @@ static int get_send_buffer_size(int sk)
>  	socklen_t optlen = sizeof(size);
>  
>  	if (getsockopt(sk, SOL_SOCKET, SO_SNDBUF, &size, &optlen) < 0) {
> -		int err = -errno;
> -		error("getsockopt(SO_SNDBUF) failed: %s (%d)", strerror(-err),
> -									-err);
> -		return err;
> +		error("getsockopt(SO_SNDBUF) failed: %s (%d)", strerror(errno),
> +									errno);
> +		return -errno;
>  	}

This is not an unneeded variable. The error() call itself might cause
the value of errno to be modified and hence you'd be returning not the
getsockopt error but something else.

Johan

[PATCH 3/5] hciemu: Make code consistent

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

It is enough to check for zero in __sync_sub_and_fetch(). This  makes
code consistent like shown below:

./src/shared/mgmt.c:    if (__sync_sub_and_fetch(&mgmt->ref_count, 1))
./src/shared/pcap.c:    if (__sync_sub_and_fetch(&pcap->ref_count, 1))
./src/shared/btsnoop.c: if (__sync_sub_and_fetch(&btsnoop->ref_count, 1))
./src/shared/hciemu.c:  if (__sync_sub_and_fetch(&hciemu->ref_count, 1))
---
 src/shared/hciemu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/hciemu.c b/src/shared/hciemu.c
index 0ea191f..c2b4748 100644
--- a/src/shared/hciemu.c
+++ b/src/shared/hciemu.c
@@ -341,7 +341,7 @@ void hciemu_unref(struct hciemu *hciemu)
 	if (!hciemu)
 		return;
 
-	if (__sync_sub_and_fetch(&hciemu->ref_count, 1) > 0)
+	if (__sync_sub_and_fetch(&hciemu->ref_count, 1))
 		return;
 
 	g_list_foreach(hciemu->post_command_hooks, destroy_command_hook, NULL);
-- 
1.8.3.2

[PATCH 4/5] l2cap-tester: Remove unneeded variable

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

err is only used to assign -errno before return.
---
 tools/l2cap-tester.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/tools/l2cap-tester.c b/tools/l2cap-tester.c
index e4dade2..4b1e5e2 100644
--- a/tools/l2cap-tester.c
+++ b/tools/l2cap-tester.c
@@ -489,15 +489,14 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
 {
 	const uint8_t *master_bdaddr;
 	struct sockaddr_l2 addr;
-	int sk, err;
+	int sk;
 
 	sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | SOCK_NONBLOCK,
 							BTPROTO_L2CAP);
 	if (sk < 0) {
-		err = -errno;
 		tester_warn("Can't create socket: %s (%d)", strerror(errno),
 									errno);
-		return err;
+		return -errno;
 	}
 
 	master_bdaddr = hciemu_get_master_bdaddr(data->hciemu);
@@ -517,11 +516,10 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
 		addr.l2_bdaddr_type = BDADDR_BREDR;
 
 	if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
-		err = -errno;
 		tester_warn("Can't bind socket: %s (%d)", strerror(errno),
 									errno);
 		close(sk);
-		return err;
+		return -errno;
 	}
 
 	return sk;
-- 
1.8.3.2

Re: [PATCH 4/5] l2cap-tester: Remove unneeded variable

[Anderson Lizardo]

Hi Andrei,

On Mon, Dec 16, 2013 at 4:57 AM, Andrei Emeltchenko
<Andrei.Emeltchenko.news@gmail.com> wrote:
> --- a/tools/l2cap-tester.c
> +++ b/tools/l2cap-tester.c
> @@ -489,15 +489,14 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
>  {
>         const uint8_t *master_bdaddr;
>         struct sockaddr_l2 addr;
> -       int sk, err;
> +       int sk;
>
>         sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | SOCK_NONBLOCK,
>                                                         BTPROTO_L2CAP);
>         if (sk < 0) {
> -               err = -errno;
>                 tester_warn("Can't create socket: %s (%d)", strerror(errno),
>                                                                         errno);
> -               return err;
> +               return -errno;
>         }
>
>         master_bdaddr = hciemu_get_master_bdaddr(data->hciemu);
> @@ -517,11 +516,10 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
>                 addr.l2_bdaddr_type = BDADDR_BREDR;
>
>         if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> -               err = -errno;
>                 tester_warn("Can't bind socket: %s (%d)", strerror(errno),
>                                                                         errno);
>                 close(sk);
> -               return err;
> +               return -errno;

This is not a good idea because close() will overwrite the original
error if it fails as well. The previous situation is also valid
because tester_warn() may call library functions that set errno.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 4/5] l2cap-tester: Remove unneeded variable

[Andrei Emeltchenko]

Hi Anderson,

On Mon, Dec 16, 2013 at 06:39:28AM -0400, Anderson Lizardo wrote:
> Hi Andrei,
> 
> On Mon, Dec 16, 2013 at 4:57 AM, Andrei Emeltchenko
> <Andrei.Emeltchenko.news@gmail.com> wrote:
> > --- a/tools/l2cap-tester.c
> > +++ b/tools/l2cap-tester.c
> > @@ -489,15 +489,14 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
> >  {
> >         const uint8_t *master_bdaddr;
> >         struct sockaddr_l2 addr;
> > -       int sk, err;
> > +       int sk;
> >
> >         sk = socket(PF_BLUETOOTH, SOCK_SEQPACKET | SOCK_NONBLOCK,
> >                                                         BTPROTO_L2CAP);
> >         if (sk < 0) {
> > -               err = -errno;
> >                 tester_warn("Can't create socket: %s (%d)", strerror(errno),
> >                                                                         errno);
> > -               return err;
> > +               return -errno;
> >         }
> >
> >         master_bdaddr = hciemu_get_master_bdaddr(data->hciemu);
> > @@ -517,11 +516,10 @@ static int create_l2cap_sock(struct test_data *data, uint16_t psm)
> >                 addr.l2_bdaddr_type = BDADDR_BREDR;
> >
> >         if (bind(sk, (struct sockaddr *) &addr, sizeof(addr)) < 0) {
> > -               err = -errno;
> >                 tester_warn("Can't bind socket: %s (%d)", strerror(errno),
> >                                                                         errno);
> >                 close(sk);
> > -               return err;
> > +               return -errno;
> 
> This is not a good idea because close() will overwrite the original
> error if it fails as well. The previous situation is also valid
> because tester_warn() may call library functions that set errno.

Yes, though the return value is not used at all, we may just return -1.

Best regards 
Andrei Emeltchenko 

[PATCH 5/5] hciemu: Print error in case hci_vhci is not loaded

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Error message should indicate that module is not loaded:
Opening /dev/vhci failed: No such file or directory
---
 src/shared/hciemu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/shared/hciemu.c b/src/shared/hciemu.c
index c2b4748..9f4bfaf 100644
--- a/src/shared/hciemu.c
+++ b/src/shared/hciemu.c
@@ -31,6 +31,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdbool.h>
+#include <errno.h>
 #include <sys/socket.h>
 
 #include <glib.h>
@@ -216,6 +217,7 @@ static bool create_vhci(struct hciemu *hciemu)
 
 	fd = open("/dev/vhci", O_RDWR | O_NONBLOCK | O_CLOEXEC);
 	if (fd < 0) {
+		perror("Opening /dev/vhci failed");
 		btdev_destroy(btdev);
 		return false;
 	}
-- 
1.8.3.2