[PATCH 1/9] android/ipc-tester: Fix possible double close

[PATCH 1/9] android/ipc-tester: Fix possible double close

[Andrei Emeltchenko]

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

I case of error we may close fd and pipe twice.
---
 android/ipc-tester.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/android/ipc-tester.c b/android/ipc-tester.c
index f1f93f2..f23e4b6 100644
--- a/android/ipc-tester.c
+++ b/android/ipc-tester.c
@@ -275,7 +275,7 @@ static void emulator(int pipe, int hci_index)
 
 	close(pipe);
 	close(fd);
-	bluetoothd_start(hci_index);
+	return bluetoothd_start(hci_index);
 
 failed:
 	close(pipe);
-- 
1.8.3.2

[PATCH 2/9] tools/mpris-player: Fix overflow before type widening

[Andrei Emeltchenko]

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

Expression is evaluated using 32-bit arithmetic before conversion to
64-bit.
---
 tools/mpris-player.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/mpris-player.c b/tools/mpris-player.c
index c94330c..397f064 100644
--- a/tools/mpris-player.c
+++ b/tools/mpris-player.c
@@ -1135,7 +1135,7 @@ static gboolean get_position(const GDBusPropertyTable *property,
 
 	dbus_message_iter_get_basic(&var, &position);
 
-	value = position * 1000;
+	value = position * 1000ll;
 
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_INT64, &value);
 
@@ -1195,7 +1195,7 @@ static gboolean parse_int64_metadata(DBusMessageIter *iter, const char *key,
 
 	dbus_message_iter_get_basic(iter, &duration);
 
-	value = duration * 1000;
+	value = duration * 1000ll;
 
 	dict_append_entry(metadata, key, DBUS_TYPE_INT64, &value);
 
@@ -2412,7 +2412,7 @@ static void player_property_changed(GDBusProxy *proxy, const char *name,
 
 	dbus_message_iter_get_basic(iter, &position);
 
-	value = position * 1000;
+	value = position * 1000ll;
 
 	g_dbus_emit_signal(player->conn, MPRIS_PLAYER_PATH,
 					MPRIS_PLAYER_INTERFACE, "Seeked",
-- 
1.8.3.2

[PATCH 3/9] queue: Fix unreachable code

[Andrei Emeltchenko]

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

I assume passing function == NULL means using direct_match() so make it
reachable.
---
 src/shared/queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/queue.c b/src/shared/queue.c
index 8bbb7df..4013293 100644
--- a/src/shared/queue.c
+++ b/src/shared/queue.c
@@ -234,7 +234,7 @@ void *queue_find(struct queue *queue, queue_match_func_t function,
 {
 	struct queue_entry *entry;
 
-	if (!queue || !function)
+	if (!queue)
 		return NULL;
 
 	if (!function)
-- 
1.8.3.2

[PATCH 4/9] tools/csr: Fix wrong error check

[Andrei Emeltchenko]

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

---
 tools/csr.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/csr.c b/tools/csr.c
index b4ea1fb..36af921 100644
--- a/tools/csr.c
+++ b/tools/csr.c
@@ -2750,10 +2750,12 @@ static int parse_line(char *str)
 	char *off, *end;
 
 	pskey = strtol(str + 1, NULL, 16);
-	off = strstr(str, "=") + 1;
+	off = strstr(str, "=");
 	if (!off)
 		return -EIO;
 
+	off++;
+
 	while (1) {
 		value = strtol(off, &end, 16);
 		if (value == 0 && off == end)
-- 
1.8.3.2

[PATCH 5/9] monitor: Fix checking boolean return value

[Andrei Emeltchenko]

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

Since btsnoop_read_hci() is used from src/shared.
---
 monitor/analyze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/monitor/analyze.c b/monitor/analyze.c
index a747938..a5ed5f4 100644
--- a/monitor/analyze.c
+++ b/monitor/analyze.c
@@ -286,8 +286,8 @@ void analyze_trace(const char *path)
 		struct timeval tv;
 		uint16_t index, opcode, pktlen;
 
-		if (btsnoop_read_hci(btsnoop_file, &tv, &index, &opcode,
-							buf, &pktlen) < 0)
+		if (!btsnoop_read_hci(btsnoop_file, &tv, &index, &opcode,
+								buf, &pktlen))
 			break;
 
 		switch (opcode) {
-- 
1.8.3.2

[PATCH 6/9] bnep: Fix redundant check

[Andrei Emeltchenko]

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

---
 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 87304c5..136709d 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -556,7 +556,7 @@ 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)
 {
-	if (!bridge || !bridge || !iface || !addr)
+	if (!bridge || !iface || !addr)
 		return -EINVAL;
 
 	if (bnep_connadd(sk, dst, iface) < 0) {
-- 
1.8.3.2

[PATCH 7/9] hcitool: Fix adding missing break

[Andrei Emeltchenko]

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

---
 tools/hcitool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/hcitool.c b/tools/hcitool.c
index ffaf953..661c96f 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -363,7 +363,8 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		if (strlen(cls_str) > 0)
 			return cls_str;
-	}
+		}
+		break;
 	case 6:	/* imaging */
 		if (minor & 4)
 			return "Display";
-- 
1.8.3.2

[PATCH 8/9] hciconfig: Fix adding missing break

[Andrei Emeltchenko]

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

---
 tools/hciconfig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index 765d980..7018379 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -856,7 +856,8 @@ static char *get_minor_device_name(int major, int minor)
 		}
 		if (strlen(cls_str) > 0)
 			return cls_str;
-	}
+		}
+		break;
 	case 6:	/* imaging */
 		if (minor & 4)
 			return "Display";
-- 
1.8.3.2

[PATCH 9/9] Fix using address of array instead of size

[Andrei Emeltchenko]

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

---
 src/device.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/device.c b/src/device.c
index a9b644b..2003c7f 100644
--- a/src/device.c
+++ b/src/device.c
@@ -595,7 +595,11 @@ static gboolean dev_property_get_name(const GDBusPropertyTable *property,
 	struct btd_device *device = data;
 	const char *empty = "", *ptr;
 
-	ptr = device->name ?: empty;
+	if (strlen(device->name) > 0)
+		ptr = device->name;
+	else
+		ptr = empty;
+
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
 
 	return TRUE;
-- 
1.8.3.2

Re: [PATCH 9/9] Fix using address of array instead of size

[Johan Hedberg]

Hi Andrei,

On Thu, Jun 19, 2014, Andrei Emeltchenko wrote:
> ---
>  src/device.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

I've pushed patches 1-8 (with fixes to 7 and 8 due to messed up
indentation). This one doesn't make much sense though:

> diff --git a/src/device.c b/src/device.c
> index a9b644b..2003c7f 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -595,7 +595,11 @@ static gboolean dev_property_get_name(const GDBusPropertyTable *property,
>  	struct btd_device *device = data;
>  	const char *empty = "", *ptr;
>  
> -	ptr = device->name ?: empty;
> +	if (strlen(device->name) > 0)
> +		ptr = device->name;
> +	else
> +		ptr = empty;

Why do you need the whole "empty" variable here? If strlen(device->name)
is 0 then you've got the exact same thing: a pointer to an empty string.
So to me it seems that this snippet should be changed to simply:

	ptr = device->name;

Johan

Re: [PATCH 9/9] Fix using address of array instead of size

[Andrei Emeltchenko]

On Mon, Jun 23, 2014 at 10:55:29AM +0300, Johan Hedberg wrote:
> Hi Andrei,
> 
> On Thu, Jun 19, 2014, Andrei Emeltchenko wrote:
> > ---
> >  src/device.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> I've pushed patches 1-8 (with fixes to 7 and 8 due to messed up
> indentation). This one doesn't make much sense though:
> 
> > diff --git a/src/device.c b/src/device.c
> > index a9b644b..2003c7f 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -595,7 +595,11 @@ static gboolean dev_property_get_name(const GDBusPropertyTable *property,
> >  	struct btd_device *device = data;
> >  	const char *empty = "", *ptr;
> >  
> > -	ptr = device->name ?: empty;
> > +	if (strlen(device->name) > 0)
> > +		ptr = device->name;
> > +	else
> > +		ptr = empty;
> 
> Why do you need the whole "empty" variable here? If strlen(device->name)
> is 0 then you've got the exact same thing: a pointer to an empty string.
> So to me it seems that this snippet should be changed to simply:
> 
> 	ptr = device->name;

Yes, correct.

Best regards 
Andrei Emeltchenko 

[PATCH] Fix using address of array instead of size

[Andrei Emeltchenko]

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

Address of array is always not NULL.
---
 src/device.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/device.c b/src/device.c
index a9b644b..2c9ff92 100644
--- a/src/device.c
+++ b/src/device.c
@@ -593,9 +593,8 @@ static gboolean dev_property_get_name(const GDBusPropertyTable *property,
 					DBusMessageIter *iter, void *data)
 {
 	struct btd_device *device = data;
-	const char *empty = "", *ptr;
+	const char *ptr = device->name;
 
-	ptr = device->name ?: empty;
 	dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &ptr);
 
 	return TRUE;
-- 
1.8.3.2

Re: [PATCH] Fix using address of array instead of size

[Johan Hedberg]

Hi Andrei,

On Mon, Jun 23, 2014, Andrei Emeltchenko wrote:
> Address of array is always not NULL.
> ---
>  src/device.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Applied. Thanks.

Johan