From: Andrei Emeltchenko <[email protected]>
---
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
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
> <[email protected]> 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
Hi Andrei,
On Mon, Dec 16, 2013 at 4:57 AM, Andrei Emeltchenko
<[email protected]> 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
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
From: Andrei Emeltchenko <[email protected]>
---
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
From: Andrei Emeltchenko <[email protected]>
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
From: Andrei Emeltchenko <[email protected]>
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
From: Andrei Emeltchenko <[email protected]>
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