From: Andrei Emeltchenko <[email protected]>
Variable rfcomm is never used after assignment
---
src/profile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/profile.c b/src/profile.c
index 528525a..7aca3be 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1272,7 +1272,6 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
error("RFCOMM server failed for %s: %s",
ext->name, err->message);
g_free(rfcomm);
- rfcomm = NULL;
g_clear_error(&err);
goto failed;
} else {
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/l2test.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/l2test.c b/tools/l2test.c
index ffad7c4..67ca70a 100644
--- a/tools/l2test.c
+++ b/tools/l2test.c
@@ -768,8 +768,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for message_listing_cb()
---
obexd/client/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 331aebc..5d7f0dc 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1182,8 +1182,10 @@ static void message_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
+ if (reply == NULL) {
+ g_free(contents);
return;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Code is unreachable since we use exit() before
---
tools/rctest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index 2c7e45b..a09a973 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -437,8 +437,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/scotest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/scotest.c b/tools/scotest.c
index e5530d9..bd65034 100644
--- a/tools/scotest.c
+++ b/tools/scotest.c
@@ -237,8 +237,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
lmpver and hciver are allocated through malloc and need to be freed.
---
tools/parser/hci.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/parser/hci.c b/tools/parser/hci.c
index 351f843..1a639af 100644
--- a/tools/parser/hci.c
+++ b/tools/parser/hci.c
@@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
p_indent(level, frm);
printf("Error: %s\n", status2str(rp->status));
} else {
+ char *lmpver = lmp_vertostr(rp->lmp_ver);
+ char *hciver = hci_vertostr(rp->hci_ver);
+
p_indent(level, frm);
printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
- hci_vertostr(rp->hci_ver),
+ hciver ? hciver : "n/a",
rp->hci_ver, btohs(rp->hci_rev));
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(rp->lmp_ver),
+ lmpver ? lmpver : "n/a",
rp->lmp_ver, btohs(rp->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ bt_free(lmpver);
+ if (hciver)
+ bt_free(hciver);
}
}
@@ -3178,13 +3186,18 @@ static inline void read_remote_version_complete_dump(int level, struct frame *fr
p_indent(level, frm);
printf("Error: %s\n", status2str(evt->status));
} else {
+ char *lmpver = lmp_vertostr(evt->lmp_ver);
+
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(evt->lmp_ver), evt->lmp_ver,
+ lmpver ? lmpver : "n/a", evt->lmp_ver,
btohs(evt->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ bt_free(lmpver);
}
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for folder_listing_cb().
---
obexd/client/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index d2d3d81..331aebc 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -271,8 +271,10 @@ static void folder_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
+ if (reply == NULL) {
+ g_free(contents);
return;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
rsp_len cannot be NULL since it is checked above and dereferenced.
Remove static analyzers warnings.
---
android/hal-ipc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 494ba85..363072c 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -462,8 +462,7 @@ int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
}
}
- if (rsp_len)
- *rsp_len = cmd.len;
+ *rsp_len = cmd.len;
return BT_STATUS_SUCCESS;
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
attrib/gatttool.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index bca7f69..db5da62 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -340,6 +340,7 @@ static gboolean characteristics_write(gpointer user_data)
gatt_write_cmd(attrib, opt_handle, value, len, mainloop_quit, value);
+ g_free(value);
return FALSE;
error:
@@ -393,6 +394,7 @@ static gboolean characteristics_write_req(gpointer user_data)
gatt_write_char(attrib, opt_handle, value, len, char_write_req_cb,
NULL);
+ g_free(value);
return FALSE;
error:
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Variable rfcomm is never used after assignment
---
src/profile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/profile.c b/src/profile.c
index 528525a..7aca3be 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1272,7 +1272,6 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
error("RFCOMM server failed for %s: %s",
ext->name, err->message);
g_free(rfcomm);
- rfcomm = NULL;
g_clear_error(&err);
goto failed;
} else {
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
When executing in a child we close(sk) in the beginning. error label
handles parent process error conditions.
---
tools/scotest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/scotest.c b/tools/scotest.c
index bd65034..d033ae0 100644
--- a/tools/scotest.c
+++ b/tools/scotest.c
@@ -210,7 +210,7 @@ static void do_listen(void (*handler)(int sk))
strerror(errno), errno);
if (!defer_setup) {
close(nsk);
- goto error;
+ exit(1);
}
}
@@ -227,7 +227,7 @@ static void do_listen(void (*handler)(int sk))
if (defer_setup < 0) {
close(nsk);
- goto error;
+ exit(1);
}
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Since do_listen() never returns remove hanging code
---
tools/rctest.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index a09a973..583b50c 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -629,19 +629,17 @@ static void automated_send_recv()
char device[18];
if (fork()) {
- if (!savefile) {
+ if (!savefile)
+ /* do_listen() never returns */
do_listen(recv_mode);
- return;
- }
save_fd = open(savefile, O_CREAT | O_WRONLY,
S_IRUSR | S_IWUSR);
if (save_fd < 0)
syslog(LOG_ERR, "Failed to open file to save data");
+ /* do_listen() never returns */
do_listen(save_mode);
-
- close(save_fd);
} else {
ba2str(&bdaddr, device);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
g_clear_error() frees err and set it to NULL. This fixes dead code
warnings from analyzers.
---
android/bluetooth.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index a2ab445..52d816f 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -290,8 +290,7 @@ static void load_adapter_config(void)
"General", "DiscoverableTimeout", &gerr);
if (gerr) {
adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
- g_error_free(gerr);
- gerr = NULL;
+ g_clear_error(&gerr);
}
g_key_file_free(key_file);
--
1.9.1
Hi Andrei,
On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> On Mon, Jul 28, 2014 at 04:57:50PM +0300, Johan Hedberg wrote:
> > Hi Andrei,
> >
> > On Mon, Jul 28, 2014, Andrei Emeltchenko wrote:
> > > ---
> > > android/bluetooth.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/android/bluetooth.c b/android/bluetooth.c
> > > index a2ab445..5b72a36 100644
> > > --- a/android/bluetooth.c
> > > +++ b/android/bluetooth.c
> > > @@ -291,7 +291,6 @@ static void load_adapter_config(void)
> > > if (gerr) {
> > > adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
> > > g_error_free(gerr);
> > > - gerr = NULL;
> > > }
> > >
> > > g_key_file_free(key_file);
> >
> > I don't think patches like this are a good idea since they make the code
> > more prone to bugs in the future. In this case if the function was ever
> > extended and gerr reused one would need to remember to "fix" the old
> > code so that it's re-initialized to NULL.
>
> This is common practice, to initialize if needed value before use
> (especially if value is reused). Makes code more clean.
Then use at least g_clear_error instead of g_error_free.
> This also fixes static analyzer warnings.
To me it looks more like this is not just a nice side effect (which you
imply by your "also ...") but the main reason you created this patch.
Johan
Hi Johan,
On Mon, Jul 28, 2014 at 04:57:50PM +0300, Johan Hedberg wrote:
> Hi Andrei,
>
> On Mon, Jul 28, 2014, Andrei Emeltchenko wrote:
> > ---
> > android/bluetooth.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/android/bluetooth.c b/android/bluetooth.c
> > index a2ab445..5b72a36 100644
> > --- a/android/bluetooth.c
> > +++ b/android/bluetooth.c
> > @@ -291,7 +291,6 @@ static void load_adapter_config(void)
> > if (gerr) {
> > adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
> > g_error_free(gerr);
> > - gerr = NULL;
> > }
> >
> > g_key_file_free(key_file);
>
> I don't think patches like this are a good idea since they make the code
> more prone to bugs in the future. In this case if the function was ever
> extended and gerr reused one would need to remember to "fix" the old
> code so that it's re-initialized to NULL.
This is common practice, to initialize if needed value before use
(especially if value is reused). Makes code more clean.
This also fixes static analyzer warnings.
Best regards
Andrei Emeltchenko
Hi Andrei,
On Mon, Jul 28, 2014, Andrei Emeltchenko wrote:
> Since do_listen() never returns remove hanging code
> ---
> tools/rctest.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/tools/rctest.c b/tools/rctest.c
> index a09a973..e61f7e2 100644
> --- a/tools/rctest.c
> +++ b/tools/rctest.c
> @@ -629,10 +629,8 @@ static void automated_send_recv()
> char device[18];
>
> if (fork()) {
> - if (!savefile) {
> + if (!savefile)
> do_listen(recv_mode);
> - return;
> - }
>
> save_fd = open(savefile, O_CREAT | O_WRONLY,
> S_IRUSR | S_IWUSR);
> @@ -640,8 +638,6 @@ static void automated_send_recv()
> syslog(LOG_ERR, "Failed to open file to save data");
>
> do_listen(save_mode);
> -
> - close(save_fd);
> } else {
> ba2str(&bdaddr, device);
I'd either forget about this patch and leave the code as-is, or then at
least add a comment before the do_listen() call to explain that it will
never return.
Johan
Hi Andrei,
On Mon, Jul 28, 2014, Andrei Emeltchenko wrote:
> ---
> android/bluetooth.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/android/bluetooth.c b/android/bluetooth.c
> index a2ab445..5b72a36 100644
> --- a/android/bluetooth.c
> +++ b/android/bluetooth.c
> @@ -291,7 +291,6 @@ static void load_adapter_config(void)
> if (gerr) {
> adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
> g_error_free(gerr);
> - gerr = NULL;
> }
>
> g_key_file_free(key_file);
I don't think patches like this are a good idea since they make the code
more prone to bugs in the future. In this case if the function was ever
extended and gerr reused one would need to remember to "fix" the old
code so that it's re-initialized to NULL.
Johan
From: Andrei Emeltchenko <[email protected]>
rsp_len cannot be NULL since it is checked above and dereferenced.
Remove static analyzers warnings.
---
android/hal-ipc.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/android/hal-ipc.c b/android/hal-ipc.c
index 494ba85..363072c 100644
--- a/android/hal-ipc.c
+++ b/android/hal-ipc.c
@@ -462,8 +462,7 @@ int hal_ipc_cmd(uint8_t service_id, uint8_t opcode, uint16_t len, void *param,
}
}
- if (rsp_len)
- *rsp_len = cmd.len;
+ *rsp_len = cmd.len;
return BT_STATUS_SUCCESS;
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/scotest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/scotest.c b/tools/scotest.c
index e5530d9..bd65034 100644
--- a/tools/scotest.c
+++ b/tools/scotest.c
@@ -237,8 +237,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Variable rfcomm is never used after assignment
---
src/profile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/profile.c b/src/profile.c
index 528525a..7aca3be 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1272,7 +1272,6 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
error("RFCOMM server failed for %s: %s",
ext->name, err->message);
g_free(rfcomm);
- rfcomm = NULL;
g_clear_error(&err);
goto failed;
} else {
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/l2test.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/l2test.c b/tools/l2test.c
index ffad7c4..67ca70a 100644
--- a/tools/l2test.c
+++ b/tools/l2test.c
@@ -768,8 +768,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Code is unreachable since we use exit() before
---
tools/rctest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index 2c7e45b..a09a973 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -437,8 +437,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for message_listing_cb()
---
obexd/client/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 331aebc..5d7f0dc 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1182,8 +1182,10 @@ static void message_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
+ if (reply == NULL) {
+ g_free(contents);
return;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for folder_listing_cb().
---
obexd/client/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index d2d3d81..331aebc 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -271,8 +271,10 @@ static void folder_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
+ if (reply == NULL) {
+ g_free(contents);
return;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
When executing in a child we close(sk) in the beginning. error label
handles parent process error conditions.
---
tools/scotest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/scotest.c b/tools/scotest.c
index bd65034..d033ae0 100644
--- a/tools/scotest.c
+++ b/tools/scotest.c
@@ -210,7 +210,7 @@ static void do_listen(void (*handler)(int sk))
strerror(errno), errno);
if (!defer_setup) {
close(nsk);
- goto error;
+ exit(1);
}
}
@@ -227,7 +227,7 @@ static void do_listen(void (*handler)(int sk))
if (defer_setup < 0) {
close(nsk);
- goto error;
+ exit(1);
}
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
lmpver and hciver are allocated through malloc and need to be freed.
---
tools/parser/hci.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/parser/hci.c b/tools/parser/hci.c
index 351f843..1a639af 100644
--- a/tools/parser/hci.c
+++ b/tools/parser/hci.c
@@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
p_indent(level, frm);
printf("Error: %s\n", status2str(rp->status));
} else {
+ char *lmpver = lmp_vertostr(rp->lmp_ver);
+ char *hciver = hci_vertostr(rp->hci_ver);
+
p_indent(level, frm);
printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
- hci_vertostr(rp->hci_ver),
+ hciver ? hciver : "n/a",
rp->hci_ver, btohs(rp->hci_rev));
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(rp->lmp_ver),
+ lmpver ? lmpver : "n/a",
rp->lmp_ver, btohs(rp->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ bt_free(lmpver);
+ if (hciver)
+ bt_free(hciver);
}
}
@@ -3178,13 +3186,18 @@ static inline void read_remote_version_complete_dump(int level, struct frame *fr
p_indent(level, frm);
printf("Error: %s\n", status2str(evt->status));
} else {
+ char *lmpver = lmp_vertostr(evt->lmp_ver);
+
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(evt->lmp_ver), evt->lmp_ver,
+ lmpver ? lmpver : "n/a", evt->lmp_ver,
btohs(evt->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ bt_free(lmpver);
}
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
android/bluetooth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index a2ab445..5b72a36 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -291,7 +291,6 @@ static void load_adapter_config(void)
if (gerr) {
adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
g_error_free(gerr);
- gerr = NULL;
}
g_key_file_free(key_file);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Since do_listen() never returns remove hanging code
---
tools/rctest.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index a09a973..e61f7e2 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -629,10 +629,8 @@ static void automated_send_recv()
char device[18];
if (fork()) {
- if (!savefile) {
+ if (!savefile)
do_listen(recv_mode);
- return;
- }
save_fd = open(savefile, O_CREAT | O_WRONLY,
S_IRUSR | S_IWUSR);
@@ -640,8 +638,6 @@ static void automated_send_recv()
syslog(LOG_ERR, "Failed to open file to save data");
do_listen(save_mode);
-
- close(save_fd);
} else {
ba2str(&bdaddr, device);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
android/bluetooth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index a2ab445..5b72a36 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -291,7 +291,6 @@ static void load_adapter_config(void)
if (gerr) {
adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
g_error_free(gerr);
- gerr = NULL;
}
g_key_file_free(key_file);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Since do_listen() never returns remove hanging code
---
tools/rctest.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index a09a973..e61f7e2 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -629,10 +629,8 @@ static void automated_send_recv()
char device[18];
if (fork()) {
- if (!savefile) {
+ if (!savefile)
do_listen(recv_mode);
- return;
- }
save_fd = open(savefile, O_CREAT | O_WRONLY,
S_IRUSR | S_IWUSR);
@@ -640,8 +638,6 @@ static void automated_send_recv()
syslog(LOG_ERR, "Failed to open file to save data");
do_listen(save_mode);
-
- close(save_fd);
} else {
ba2str(&bdaddr, device);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Code is unreachable since we use exit() before
---
tools/rctest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/rctest.c b/tools/rctest.c
index 2c7e45b..a09a973 100644
--- a/tools/rctest.c
+++ b/tools/rctest.c
@@ -437,8 +437,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/scotest.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/scotest.c b/tools/scotest.c
index e5530d9..bd65034 100644
--- a/tools/scotest.c
+++ b/tools/scotest.c
@@ -237,8 +237,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Variable rfcomm is never used after assignment
---
src/profile.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/profile.c b/src/profile.c
index 528525a..7aca3be 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -1272,7 +1272,6 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
error("RFCOMM server failed for %s: %s",
ext->name, err->message);
g_free(rfcomm);
- rfcomm = NULL;
g_clear_error(&err);
goto failed;
} else {
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Remove dead code after exit()
---
tools/l2test.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/l2test.c b/tools/l2test.c
index ffad7c4..67ca70a 100644
--- a/tools/l2test.c
+++ b/tools/l2test.c
@@ -768,8 +768,6 @@ static void do_listen(void (*handler)(int sk))
exit(0);
}
- return;
-
error:
close(sk);
exit(1);
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
---
android/bluetooth.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/android/bluetooth.c b/android/bluetooth.c
index a2ab445..5b72a36 100644
--- a/android/bluetooth.c
+++ b/android/bluetooth.c
@@ -291,7 +291,6 @@ static void load_adapter_config(void)
if (gerr) {
adapter.discoverable_timeout = DEFAULT_DISCOVERABLE_TIMEOUT;
g_error_free(gerr);
- gerr = NULL;
}
g_key_file_free(key_file);
--
1.9.1
Hi Andrei,
On Fri, Aug 01, 2014, Andrei Emeltchenko wrote:
> lmpver and hciver are allocated through malloc and need to be freed.
> ---
> tools/parser/hci.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
All three patches have been applied. Thanks.
Johan
Hi Johan,
On Fri, Aug 01, 2014 at 02:33:10PM +0300, Johan Hedberg wrote:
> > > > > > + if (lmpver)
> > > > > > + bt_free(lmpver);
> > > > > > + if (hciver)
> > > > > > + bt_free(hciver);
> > > > >
> > > > > These trace back to using malloc (in hci_uint2str) so I suppose free is
> > > > > more appropriate than bt_free (which should be used for bt_malloc).
> > > >
> > > > OK, I will change it to free(). Shall I also change other similar
> > > > bt_free() calls which I took as example?
> > >
> > > I'm not sure it's worth bothering with legacy code like this which will
> > > eventually disappear from the tree.
> >
> > Those constructions are in hcitool and hciconfig
>
> Still legacy code. If you really feel like there's nothing else
> important to work on feel free to send patches for that.
The question is if we change some programming practice shall we make other
code looks consistent.
Best regards
Andrei Emeltchenko
Hi Andrei,
On Fri, Aug 01, 2014, Andrei Emeltchenko wrote:
> On Fri, Aug 01, 2014 at 02:21:24PM +0300, Johan Hedberg wrote:
> > Hi Andrei,
> >
> > On Fri, Aug 01, 2014, Andrei Emeltchenko wrote:
> > > Hi Johan,
> > >
> > > On Fri, Aug 01, 2014 at 01:35:17PM +0300, Johan Hedberg wrote:
> > > > Hi Andrei,
> > > >
> > > > On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> > > > > lmpver and hciver are allocated through malloc and need to be freed.
> > > > > ---
> > > > > tools/parser/hci.c | 19 ++++++++++++++++---
> > > > > 1 file changed, 16 insertions(+), 3 deletions(-)
> > > >
> > > > I've applied all 8 patches before this one.
> > > >
> > > > > diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> > > > > index 351f843..1a639af 100644
> > > > > --- a/tools/parser/hci.c
> > > > > +++ b/tools/parser/hci.c
> > > > > @@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
> > > > > p_indent(level, frm);
> > > > > printf("Error: %s\n", status2str(rp->status));
> > > > > } else {
> > > > > + char *lmpver = lmp_vertostr(rp->lmp_ver);
> > > > > + char *hciver = hci_vertostr(rp->hci_ver);
> > > > > +
> > > > > p_indent(level, frm);
> > > > > printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
> > > > > - hci_vertostr(rp->hci_ver),
> > > > > + hciver ? hciver : "n/a",
> > > > > rp->hci_ver, btohs(rp->hci_rev));
> > > > > p_indent(level, frm);
> > > > > printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
> > > > > - lmp_vertostr(rp->lmp_ver),
> > > > > + lmpver ? lmpver : "n/a",
> > > > > rp->lmp_ver, btohs(rp->lmp_subver));
> > > > > p_indent(level, frm);
> > > > > printf("Manufacturer: %s (%d)\n",
> > > > > bt_compidtostr(manufacturer), manufacturer);
> > > > > +
> > > > > + if (lmpver)
> > > > > + bt_free(lmpver);
> > > > > + if (hciver)
> > > > > + bt_free(hciver);
> > > >
> > > > These trace back to using malloc (in hci_uint2str) so I suppose free is
> > > > more appropriate than bt_free (which should be used for bt_malloc).
> > >
> > > OK, I will change it to free(). Shall I also change other similar
> > > bt_free() calls which I took as example?
> >
> > I'm not sure it's worth bothering with legacy code like this which will
> > eventually disappear from the tree.
>
> Those constructions are in hcitool and hciconfig
Still legacy code. If you really feel like there's nothing else
important to work on feel free to send patches for that.
Johan
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for message_listing_cb()
---
obexd/client/map.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 0ef5e0f..47afc31 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1183,8 +1183,10 @@ static void message_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
- return;
+ if (reply == NULL) {
+ g_free(contents);
+ goto clean;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
@@ -1211,6 +1213,7 @@ static void message_listing_cb(struct obc_session *session,
done:
g_dbus_send_message(conn, reply);
+clean:
pending_request_free(request);
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for folder_listing_cb().
---
obexd/client/map.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index d2d3d81..0ef5e0f 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -271,8 +271,10 @@ static void folder_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
- return;
+ if (reply == NULL) {
+ g_free(contents);
+ goto clean;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
@@ -288,6 +290,7 @@ static void folder_listing_cb(struct obc_session *session,
done:
g_dbus_send_message(conn, reply);
+clean:
pending_request_free(request);
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
lmpver and hciver are allocated through malloc and need to be freed.
---
tools/parser/hci.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/parser/hci.c b/tools/parser/hci.c
index 351f843..cd52cb5 100644
--- a/tools/parser/hci.c
+++ b/tools/parser/hci.c
@@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
p_indent(level, frm);
printf("Error: %s\n", status2str(rp->status));
} else {
+ char *lmpver = lmp_vertostr(rp->lmp_ver);
+ char *hciver = hci_vertostr(rp->hci_ver);
+
p_indent(level, frm);
printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
- hci_vertostr(rp->hci_ver),
+ hciver ? hciver : "n/a",
rp->hci_ver, btohs(rp->hci_rev));
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(rp->lmp_ver),
+ lmpver ? lmpver : "n/a",
rp->lmp_ver, btohs(rp->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ free(lmpver);
+ if (hciver)
+ free(hciver);
}
}
@@ -3178,13 +3186,18 @@ static inline void read_remote_version_complete_dump(int level, struct frame *fr
p_indent(level, frm);
printf("Error: %s\n", status2str(evt->status));
} else {
+ char *lmpver = lmp_vertostr(evt->lmp_ver);
+
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(evt->lmp_ver), evt->lmp_ver,
+ lmpver ? lmpver : "n/a", evt->lmp_ver,
btohs(evt->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ free(lmpver);
}
}
--
1.9.1
Hi Johan,
On Fri, Aug 01, 2014 at 02:21:24PM +0300, Johan Hedberg wrote:
> Hi Andrei,
>
> On Fri, Aug 01, 2014, Andrei Emeltchenko wrote:
> > Hi Johan,
> >
> > On Fri, Aug 01, 2014 at 01:35:17PM +0300, Johan Hedberg wrote:
> > > Hi Andrei,
> > >
> > > On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> > > > lmpver and hciver are allocated through malloc and need to be freed.
> > > > ---
> > > > tools/parser/hci.c | 19 ++++++++++++++++---
> > > > 1 file changed, 16 insertions(+), 3 deletions(-)
> > >
> > > I've applied all 8 patches before this one.
> > >
> > > > diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> > > > index 351f843..1a639af 100644
> > > > --- a/tools/parser/hci.c
> > > > +++ b/tools/parser/hci.c
> > > > @@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
> > > > p_indent(level, frm);
> > > > printf("Error: %s\n", status2str(rp->status));
> > > > } else {
> > > > + char *lmpver = lmp_vertostr(rp->lmp_ver);
> > > > + char *hciver = hci_vertostr(rp->hci_ver);
> > > > +
> > > > p_indent(level, frm);
> > > > printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
> > > > - hci_vertostr(rp->hci_ver),
> > > > + hciver ? hciver : "n/a",
> > > > rp->hci_ver, btohs(rp->hci_rev));
> > > > p_indent(level, frm);
> > > > printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
> > > > - lmp_vertostr(rp->lmp_ver),
> > > > + lmpver ? lmpver : "n/a",
> > > > rp->lmp_ver, btohs(rp->lmp_subver));
> > > > p_indent(level, frm);
> > > > printf("Manufacturer: %s (%d)\n",
> > > > bt_compidtostr(manufacturer), manufacturer);
> > > > +
> > > > + if (lmpver)
> > > > + bt_free(lmpver);
> > > > + if (hciver)
> > > > + bt_free(hciver);
> > >
> > > These trace back to using malloc (in hci_uint2str) so I suppose free is
> > > more appropriate than bt_free (which should be used for bt_malloc).
> >
> > OK, I will change it to free(). Shall I also change other similar
> > bt_free() calls which I took as example?
>
> I'm not sure it's worth bothering with legacy code like this which will
> eventually disappear from the tree.
Those constructions are in hcitool and hciconfig
Best regards
Andrei Emeltchenko
Hi Andrei,
On Fri, Aug 01, 2014, Andrei Emeltchenko wrote:
> Hi Johan,
>
> On Fri, Aug 01, 2014 at 01:35:17PM +0300, Johan Hedberg wrote:
> > Hi Andrei,
> >
> > On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> > > lmpver and hciver are allocated through malloc and need to be freed.
> > > ---
> > > tools/parser/hci.c | 19 ++++++++++++++++---
> > > 1 file changed, 16 insertions(+), 3 deletions(-)
> >
> > I've applied all 8 patches before this one.
> >
> > > diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> > > index 351f843..1a639af 100644
> > > --- a/tools/parser/hci.c
> > > +++ b/tools/parser/hci.c
> > > @@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
> > > p_indent(level, frm);
> > > printf("Error: %s\n", status2str(rp->status));
> > > } else {
> > > + char *lmpver = lmp_vertostr(rp->lmp_ver);
> > > + char *hciver = hci_vertostr(rp->hci_ver);
> > > +
> > > p_indent(level, frm);
> > > printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
> > > - hci_vertostr(rp->hci_ver),
> > > + hciver ? hciver : "n/a",
> > > rp->hci_ver, btohs(rp->hci_rev));
> > > p_indent(level, frm);
> > > printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
> > > - lmp_vertostr(rp->lmp_ver),
> > > + lmpver ? lmpver : "n/a",
> > > rp->lmp_ver, btohs(rp->lmp_subver));
> > > p_indent(level, frm);
> > > printf("Manufacturer: %s (%d)\n",
> > > bt_compidtostr(manufacturer), manufacturer);
> > > +
> > > + if (lmpver)
> > > + bt_free(lmpver);
> > > + if (hciver)
> > > + bt_free(hciver);
> >
> > These trace back to using malloc (in hci_uint2str) so I suppose free is
> > more appropriate than bt_free (which should be used for bt_malloc).
>
> OK, I will change it to free(). Shall I also change other similar
> bt_free() calls which I took as example?
I'm not sure it's worth bothering with legacy code like this which will
eventually disappear from the tree.
Johan
From: Andrei Emeltchenko <[email protected]>
lmpver and hciver are allocated through malloc and need to be freed.
---
tools/parser/hci.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/tools/parser/hci.c b/tools/parser/hci.c
index 351f843..cd52cb5 100644
--- a/tools/parser/hci.c
+++ b/tools/parser/hci.c
@@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
p_indent(level, frm);
printf("Error: %s\n", status2str(rp->status));
} else {
+ char *lmpver = lmp_vertostr(rp->lmp_ver);
+ char *hciver = hci_vertostr(rp->hci_ver);
+
p_indent(level, frm);
printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
- hci_vertostr(rp->hci_ver),
+ hciver ? hciver : "n/a",
rp->hci_ver, btohs(rp->hci_rev));
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(rp->lmp_ver),
+ lmpver ? lmpver : "n/a",
rp->lmp_ver, btohs(rp->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ free(lmpver);
+ if (hciver)
+ free(hciver);
}
}
@@ -3178,13 +3186,18 @@ static inline void read_remote_version_complete_dump(int level, struct frame *fr
p_indent(level, frm);
printf("Error: %s\n", status2str(evt->status));
} else {
+ char *lmpver = lmp_vertostr(evt->lmp_ver);
+
p_indent(level, frm);
printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
- lmp_vertostr(evt->lmp_ver), evt->lmp_ver,
+ lmpver ? lmpver : "n/a", evt->lmp_ver,
btohs(evt->lmp_subver));
p_indent(level, frm);
printf("Manufacturer: %s (%d)\n",
bt_compidtostr(manufacturer), manufacturer);
+
+ if (lmpver)
+ free(lmpver);
}
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for folder_listing_cb().
---
obexd/client/map.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index d2d3d81..0ef5e0f 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -271,8 +271,10 @@ static void folder_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
- return;
+ if (reply == NULL) {
+ g_free(contents);
+ goto clean;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
@@ -288,6 +290,7 @@ static void folder_listing_cb(struct obc_session *session,
done:
g_dbus_send_message(conn, reply);
+clean:
pending_request_free(request);
}
--
1.9.1
From: Andrei Emeltchenko <[email protected]>
Fixes memory leak for message_listing_cb()
---
obexd/client/map.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/obexd/client/map.c b/obexd/client/map.c
index 0ef5e0f..47afc31 100644
--- a/obexd/client/map.c
+++ b/obexd/client/map.c
@@ -1183,8 +1183,10 @@ static void message_listing_cb(struct obc_session *session,
}
reply = dbus_message_new_method_return(request->msg);
- if (reply == NULL)
- return;
+ if (reply == NULL) {
+ g_free(contents);
+ goto clean;
+ }
dbus_message_iter_init_append(reply, &iter);
dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
@@ -1211,6 +1213,7 @@ static void message_listing_cb(struct obc_session *session,
done:
g_dbus_send_message(conn, reply);
+clean:
pending_request_free(request);
}
--
1.9.1
Hi Johan,
On Fri, Aug 01, 2014 at 01:35:17PM +0300, Johan Hedberg wrote:
> Hi Andrei,
>
> On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> > lmpver and hciver are allocated through malloc and need to be freed.
> > ---
> > tools/parser/hci.c | 19 ++++++++++++++++---
> > 1 file changed, 16 insertions(+), 3 deletions(-)
>
> I've applied all 8 patches before this one.
>
> > diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> > index 351f843..1a639af 100644
> > --- a/tools/parser/hci.c
> > +++ b/tools/parser/hci.c
> > @@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
> > p_indent(level, frm);
> > printf("Error: %s\n", status2str(rp->status));
> > } else {
> > + char *lmpver = lmp_vertostr(rp->lmp_ver);
> > + char *hciver = hci_vertostr(rp->hci_ver);
> > +
> > p_indent(level, frm);
> > printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
> > - hci_vertostr(rp->hci_ver),
> > + hciver ? hciver : "n/a",
> > rp->hci_ver, btohs(rp->hci_rev));
> > p_indent(level, frm);
> > printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
> > - lmp_vertostr(rp->lmp_ver),
> > + lmpver ? lmpver : "n/a",
> > rp->lmp_ver, btohs(rp->lmp_subver));
> > p_indent(level, frm);
> > printf("Manufacturer: %s (%d)\n",
> > bt_compidtostr(manufacturer), manufacturer);
> > +
> > + if (lmpver)
> > + bt_free(lmpver);
> > + if (hciver)
> > + bt_free(hciver);
>
> These trace back to using malloc (in hci_uint2str) so I suppose free is
> more appropriate than bt_free (which should be used for bt_malloc).
OK, I will change it to free(). Shall I also change other similar
bt_free() calls which I took as example?
Best regards
Andrei Emeltchenko
Hi Andrei,
On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> ---
> attrib/gatttool.c | 2 ++
> 1 file changed, 2 insertions(+)
This patch has also been applied. Thanks.
Johan
Hi Andrei,
On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> Fixes memory leak for message_listing_cb()
> ---
> obexd/client/map.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index 331aebc..5d7f0dc 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -1182,8 +1182,10 @@ static void message_listing_cb(struct obc_session *session,
> }
>
> reply = dbus_message_new_method_return(request->msg);
> - if (reply == NULL)
> + if (reply == NULL) {
> + g_free(contents);
> return;
> + }
>
Here also there seems to be a missing call to pending_request_free().
Might be simplest if you simply replace the return with goto done and
then put the g_dbus_send_message call behind a NULL check (same applies
to the previous patch).
Johan
Hi Andrei,
On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> Fixes memory leak for folder_listing_cb().
> ---
> obexd/client/map.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/obexd/client/map.c b/obexd/client/map.c
> index d2d3d81..331aebc 100644
> --- a/obexd/client/map.c
> +++ b/obexd/client/map.c
> @@ -271,8 +271,10 @@ static void folder_listing_cb(struct obc_session *session,
> }
>
> reply = dbus_message_new_method_return(request->msg);
> - if (reply == NULL)
> + if (reply == NULL) {
> + g_free(contents);
> return;
> + }
Shouldn't you call pending_requesT_free(request) before returning?
Johan
Hi Andrei,
On Tue, Jul 29, 2014, Andrei Emeltchenko wrote:
> lmpver and hciver are allocated through malloc and need to be freed.
> ---
> tools/parser/hci.c | 19 ++++++++++++++++---
> 1 file changed, 16 insertions(+), 3 deletions(-)
I've applied all 8 patches before this one.
> diff --git a/tools/parser/hci.c b/tools/parser/hci.c
> index 351f843..1a639af 100644
> --- a/tools/parser/hci.c
> +++ b/tools/parser/hci.c
> @@ -2445,17 +2445,25 @@ static inline void read_local_version_dump(int level, struct frame *frm)
> p_indent(level, frm);
> printf("Error: %s\n", status2str(rp->status));
> } else {
> + char *lmpver = lmp_vertostr(rp->lmp_ver);
> + char *hciver = hci_vertostr(rp->hci_ver);
> +
> p_indent(level, frm);
> printf("HCI Version: %s (0x%x) HCI Revision: 0x%x\n",
> - hci_vertostr(rp->hci_ver),
> + hciver ? hciver : "n/a",
> rp->hci_ver, btohs(rp->hci_rev));
> p_indent(level, frm);
> printf("LMP Version: %s (0x%x) LMP Subversion: 0x%x\n",
> - lmp_vertostr(rp->lmp_ver),
> + lmpver ? lmpver : "n/a",
> rp->lmp_ver, btohs(rp->lmp_subver));
> p_indent(level, frm);
> printf("Manufacturer: %s (%d)\n",
> bt_compidtostr(manufacturer), manufacturer);
> +
> + if (lmpver)
> + bt_free(lmpver);
> + if (hciver)
> + bt_free(hciver);
These trace back to using malloc (in hci_uint2str) so I suppose free is
more appropriate than bt_free (which should be used for bt_malloc).
Johan
ping
On Tue, Jul 29, 2014 at 05:20:50PM +0300, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> Variable rfcomm is never used after assignment
> ---
> src/profile.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/profile.c b/src/profile.c
> index 528525a..7aca3be 100644
> --- a/src/profile.c
> +++ b/src/profile.c
> @@ -1272,7 +1272,6 @@ static uint32_t ext_start_servers(struct ext_profile *ext,
> error("RFCOMM server failed for %s: %s",
> ext->name, err->message);
> g_free(rfcomm);
> - rfcomm = NULL;
> g_clear_error(&err);
> goto failed;
> } else {
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html