2014-10-17 12:54:05

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 1/6] bnep: Avoid double error print for bnep_connadd()

From: Andrei Emeltchenko <[email protected]>

This avoids double printing the same error with bnep connection add
ioctl.
---
profiles/network/bnep.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 136709d..035beb1 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -316,10 +316,8 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));

sk = g_io_channel_unix_get_fd(session->io);
- if (bnep_connadd(sk, session->src, session->iface)) {
- error("bnep conn could not be added");
+ if (bnep_connadd(sk, session->src, session->iface) < 0)
goto failed;
- }

if (bnep_if_up(session->iface)) {
error("could not up %s", session->iface);
@@ -556,14 +554,14 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
const bdaddr_t *addr)
{
+ int err;
+
if (!bridge || !iface || !addr)
return -EINVAL;

- if (bnep_connadd(sk, dst, iface) < 0) {
- error("Can't add connection to the bridge %s: %s(%d)",
- bridge, strerror(errno), errno);
- return -errno;
- }
+ err = bnep_connadd(sk, dst, iface);
+ if (err < 0)
+ return err;

if (bnep_add_to_bridge(iface, bridge) < 0) {
error("Can't add %s to the bridge %s: %s(%d)",
--
1.9.1



2014-10-20 12:37:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/6] bnep: Avoid double error print for bnep_connadd()

Hi Andrei,

On Fri, Oct 17, 2014 at 3:54 PM, Andrei Emeltchenko
<[email protected]> wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> This avoids double printing the same error with bnep connection add
> ioctl.
> ---
> profiles/network/bnep.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
> index 136709d..035beb1 100644
> --- a/profiles/network/bnep.c
> +++ b/profiles/network/bnep.c
> @@ -316,10 +316,8 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
> setsockopt(sk, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo));
>
> sk = g_io_channel_unix_get_fd(session->io);
> - if (bnep_connadd(sk, session->src, session->iface)) {
> - error("bnep conn could not be added");
> + if (bnep_connadd(sk, session->src, session->iface) < 0)
> goto failed;
> - }
>
> if (bnep_if_up(session->iface)) {
> error("could not up %s", session->iface);
> @@ -556,14 +554,14 @@ static int bnep_del_from_bridge(const char *devname, const char *bridge)
> int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
> const bdaddr_t *addr)
> {
> + int err;
> +
> if (!bridge || !iface || !addr)
> return -EINVAL;
>
> - if (bnep_connadd(sk, dst, iface) < 0) {
> - error("Can't add connection to the bridge %s: %s(%d)",
> - bridge, strerror(errno), errno);
> - return -errno;
> - }
> + err = bnep_connadd(sk, dst, iface);
> + if (err < 0)
> + return err;
>
> if (bnep_add_to_bridge(iface, bridge) < 0) {
> error("Can't add %s to the bridge %s: %s(%d)",
> --
> 1.9.1

Applied, thanks.

--
Luiz Augusto von Dentz

2014-10-17 12:54:10

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 6/6] bnep: Fix bnep_add_to_bridge() errno usage

From: Andrei Emeltchenko <[email protected]>

Avoid errno be overwritten and make code consistent.
---
profiles/network/bnep.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 18051c3..c40ed69 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -491,7 +491,7 @@ static int bnep_add_to_bridge(const char *devname, const char *bridge)
{
int ifindex;
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;

if (!devname || !bridge)
return -EINVAL;
@@ -506,16 +506,16 @@ static int bnep_add_to_bridge(const char *devname, const char *bridge)
strncpy(ifr.ifr_name, bridge, IFNAMSIZ - 1);
ifr.ifr_ifindex = ifindex;

- err = ioctl(sk, SIOCBRADDIF, &ifr);
+ if (ioctl(sk, SIOCBRADDIF, &ifr) < 0) {
+ err = -errno;
+ error("bnep: Can't add %s to the bridge %s: %s(%d)",
+ devname, bridge, strerror(-err), -err);
+ } else
+ info("bridge %s: interface %s added", bridge, devname);

close(sk);

- if (err < 0)
- return err;
-
- info("bridge %s: interface %s added", bridge, devname);
-
- return 0;
+ return err;
}

static int bnep_del_from_bridge(const char *devname, const char *bridge)
@@ -561,11 +561,10 @@ int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
if (err < 0)
return err;

- if (bnep_add_to_bridge(iface, bridge) < 0) {
- error("bnep: Can't add %s to the bridge %s: %s(%d)",
- iface, bridge, strerror(errno), errno);
+ err = bnep_add_to_bridge(iface, bridge);
+ if (err < 0) {
bnep_conndel(addr);
- return -errno;
+ return err;
}

return bnep_if_up(iface);
--
1.9.1


2014-10-17 12:54:08

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 4/6] bnep: Fix incorrect errno use and avoid double printing

From: Andrei Emeltchenko <[email protected]>

Fixes bnep_if_up() usage since it already prints error message and
returns errno.
---
profiles/network/bnep.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 3a6e3a4..b8d2985 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -319,7 +319,6 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
goto failed;

if (bnep_if_up(session->iface)) {
- error("bnep: could not up %s", session->iface);
bnep_conndel(&session->dst_addr);
goto failed;
}
@@ -569,13 +568,7 @@ int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
return -errno;
}

- if (bnep_if_up(iface) < 0) {
- error("bnep: Can't up the interface %s: %s(%d)",
- iface, strerror(errno), errno);
- return -errno;
- }
-
- return 0;
+ return bnep_if_up(iface);
}

void bnep_server_delete(char *bridge, char *iface, const bdaddr_t *addr)
--
1.9.1


2014-10-17 12:54:06

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 2/6] bnep: Make error logging more descriptive

From: Andrei Emeltchenko <[email protected]>

Add "bnep" before error message.
---
profiles/network/bnep.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 035beb1..e17a130 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -140,7 +140,7 @@ int bnep_init(void)
if (err == -EPROTONOSUPPORT)
warn("kernel lacks bnep-protocol support");
else
- error("Failed to open control socket: %s (%d)",
+ error("bnep: Failed to open control socket: %s (%d)",
strerror(-err), -err);

return err;
@@ -164,7 +164,7 @@ static int bnep_conndel(const bdaddr_t *dst)
req.flags = 0;
if (ioctl(ctl, BNEPCONNDEL, &req)) {
int err = -errno;
- error("Failed to kill connection: %s (%d)",
+ error("bnep: Failed to kill connection: %s (%d)",
strerror(-err), -err);
return err;
}
@@ -183,7 +183,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
req.role = role;
if (ioctl(ctl, BNEPCONNADD, &req) < 0) {
int err = -errno;
- error("Failed to add device %s: %s(%d)",
+ error("bnep: Failed to add device %s: %s(%d)",
dev, strerror(-err), -err);
return err;
}
@@ -210,7 +210,7 @@ static int bnep_if_up(const char *devname)
close(sk);

if (err < 0) {
- error("Could not bring up %s", devname);
+ error("bnep: Could not bring up %s", devname);
return err;
}

@@ -235,7 +235,7 @@ static int bnep_if_down(const char *devname)
close(sk);

if (err < 0) {
- error("Could not bring down %s", devname);
+ error("bnep: Could not bring down %s", devname);
return err;
}

@@ -272,7 +272,7 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
}

if (cond & (G_IO_HUP | G_IO_ERR)) {
- error("Hangup or error on l2cap server socket");
+ error("bnep: Hangup or error on l2cap server socket");
goto failed;
}

@@ -280,25 +280,25 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
memset(pkt, 0, BNEP_MTU);
r = read(sk, pkt, sizeof(pkt) - 1);
if (r < 0) {
- error("IO Channel read error");
+ error("bnep: IO Channel read error");
goto failed;
}

if (r == 0) {
- error("No packet received on l2cap socket");
+ error("bnep: No packet received on l2cap socket");
goto failed;
}

errno = EPROTO;

if ((size_t) r < sizeof(*rsp)) {
- error("Packet received is not bnep type");
+ error("bnep: Packet received is not bnep type");
goto failed;
}

rsp = (void *) pkt;
if (rsp->type != BNEP_CONTROL) {
- error("Packet received is not bnep type");
+ error("bnep: Packet received is not bnep type");
goto failed;
}

@@ -307,7 +307,7 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,

r = ntohs(rsp->resp);
if (r != BNEP_SUCCESS) {
- error("bnep failed");
+ error("bnep: failed");
goto failed;
}

@@ -320,7 +320,7 @@ static gboolean bnep_setup_cb(GIOChannel *chan, GIOCondition cond,
goto failed;

if (bnep_if_up(session->iface)) {
- error("could not up %s", session->iface);
+ error("bnep: could not up %s", session->iface);
bnep_conndel(&session->dst_addr);
goto failed;
}
@@ -359,7 +359,7 @@ static int bnep_setup_conn_req(struct bnep *session)

fd = g_io_channel_unix_get_fd(session->io);
if (write(fd, pkt, sizeof(*req) + sizeof(*s)) < 0) {
- error("bnep connection req send failed: %s", strerror(errno));
+ error("bnep: connection req send failed: %s", strerror(errno));
return -errno;
}

@@ -373,9 +373,9 @@ static gboolean bnep_conn_req_to(gpointer user_data)
struct bnep *session = user_data;

if (session->attempts == CON_SETUP_RETRIES) {
- error("Too many bnep connection attempts");
+ error("bnep: Too many bnep connection attempts");
} else {
- error("bnep connection setup TO, retrying...");
+ error("bnep: connection setup TO, retrying...");
if (bnep_setup_conn_req(session) == 0)
return TRUE;
}
@@ -444,7 +444,7 @@ int bnep_connect(struct bnep *session, bnep_connect_cb conn_cb, void *data)
bt_io_get(session->io, &gerr, BT_IO_OPT_DEST_BDADDR, &session->dst_addr,
BT_IO_OPT_INVALID);
if (gerr) {
- error("%s", gerr->message);
+ error("bnep: connect failed: %s", gerr->message);
g_error_free(gerr);
return -EINVAL;
}
@@ -564,14 +564,14 @@ int bnep_server_add(int sk, uint16_t dst, char *bridge, char *iface,
return err;

if (bnep_add_to_bridge(iface, bridge) < 0) {
- error("Can't add %s to the bridge %s: %s(%d)",
+ error("bnep: Can't add %s to the bridge %s: %s(%d)",
iface, bridge, strerror(errno), errno);
bnep_conndel(addr);
return -errno;
}

if (bnep_if_up(iface) < 0) {
- error("Can't up the interface %s: %s(%d)",
+ error("bnep: Can't up the interface %s: %s(%d)",
iface, strerror(errno), errno);
return -errno;
}
--
1.9.1


2014-10-17 12:54:09

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 5/6] bnep: Fix incorrect ioctl() check

From: Andrei Emeltchenko <[email protected]>

---
profiles/network/bnep.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index b8d2985..18051c3 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -162,7 +162,7 @@ static int bnep_conndel(const bdaddr_t *dst)
memset(&req, 0, sizeof(req));
baswap((bdaddr_t *)&req.dst, dst);
req.flags = 0;
- if (ioctl(ctl, BNEPCONNDEL, &req)) {
+ if (ioctl(ctl, BNEPCONNDEL, &req) < 0) {
int err = -errno;
error("bnep: Failed to kill connection: %s (%d)",
strerror(-err), -err);
--
1.9.1


2014-10-17 12:54:07

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 3/6] bnep: Refactor bnep_if_up() returning -errno

From: Andrei Emeltchenko <[email protected]>

Some functions are using bnep_if_up() like bnep_server_add() referring
directly to errno which may be overwritten already.
---
profiles/network/bnep.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index e17a130..3a6e3a4 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -195,7 +195,7 @@ static int bnep_connadd(int sk, uint16_t role, char *dev)
static int bnep_if_up(const char *devname)
{
struct ifreq ifr;
- int sk, err;
+ int sk, err = 0;

sk = socket(AF_INET, SOCK_DGRAM, 0);

@@ -205,16 +205,15 @@ static int bnep_if_up(const char *devname)
ifr.ifr_flags |= IFF_UP;
ifr.ifr_flags |= IFF_MULTICAST;

- err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
+ if (ioctl(sk, SIOCSIFFLAGS, (void *) &ifr) < 0) {
+ err = -errno;
+ error("bnep: Could not bring up %s: %s(%d)",
+ devname, strerror(-err), -err);
+ }

close(sk);

- if (err < 0) {
- error("bnep: Could not bring up %s", devname);
- return err;
- }
-
- return 0;
+ return err;
}

static int bnep_if_down(const char *devname)
--
1.9.1