2014-06-19 13:55:28

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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



2014-06-23 11:38:27

by Johan Hedberg

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

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

2014-06-23 11:19:35

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH] Fix using address of array instead of size

From: Andrei Emeltchenko <[email protected]>

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


2014-06-23 08:09:38

by Andrei Emeltchenko

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

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

2014-06-23 07:55:29

by Johan Hedberg

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

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

2014-06-19 13:55:36

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 9/9] Fix using address of array instead of size

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-19 13:55:33

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 6/9] bnep: Fix redundant 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 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


2014-06-19 13:55:31

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-19 13:55:29

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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


2014-06-19 13:55:34

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-19 13:55:32

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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


2014-06-19 13:55:35

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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


2014-06-19 13:55:30

by Andrei Emeltchenko

[permalink] [raw]
Subject: [PATCH 3/9] queue: Fix unreachable code

From: Andrei Emeltchenko <[email protected]>

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