[PATCH 1/6] Fix more memory leaks in hcitool

[PATCH 1/6] Fix more memory leaks in hcitool

[Gustavo F. Padovan]

Free some mallocs.
---
 tools/hcitool.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/tools/hcitool.c b/tools/hcitool.c
index faf4cb4..3ba9234 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -110,6 +110,7 @@ static int conn_list(int s, int dev_id, long arg)
 			addr, ci->handle, ci->state, str);
 		bt_free(str);
 	}
+	bt_free(cl);
 
 	return 0;
 }
@@ -134,9 +135,12 @@ static int find_conn(int s, int dev_id, long arg)
 	}
 
 	for (i = 0; i < cl->conn_num; i++, ci++)
-		if (!bacmp((bdaddr_t *) arg, &ci->bdaddr))
+		if (!bacmp((bdaddr_t *) arg, &ci->bdaddr)) {
+			bt_free(cl);
 			return 1;
+		}
 
+	bt_free(cl);
 	return 0;
 }
 
@@ -894,6 +898,7 @@ static void cmd_info(int dev_id, int argc, char **argv)
 		cc = 1;
 	} else
 		handle = htobs(cr->conn_info->handle);
+	bt_free(cr);
 
 	printf("\tBD Address:  %s\n", argv[0]);
 
-- 
1.6.0.6

[PATCH 2/6] hcitool: fix error message

[Gustavo F. Padovan]

str only will be NULL if malloc couldn't alloc memory
---
 tools/hcitool.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/hcitool.c b/tools/hcitool.c
index 3ba9234..4793c46 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -1861,7 +1861,7 @@ static void cmd_lp(int dev_id, int argc, char **argv)
 			printf("Link policy settings: %s\n", str);
 			bt_free(str);
 		} else {
-			fprintf(stderr, "Invalig settings\n");
+			fprintf(stderr, "Can't allocate memory\n");
 			exit(1);
 		}
 	} else {
-- 
1.6.0.6

[PATCH 3/6] Change fprintf(stderr,...) to perror()

[Gustavo F. Padovan]

Clean up code. Now hcitool use only perror to show errors messages
without  parameters.
---
 tools/hcitool.c |   38 +++++++++++++++++++-------------------
 1 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/tools/hcitool.c b/tools/hcitool.c
index 4793c46..9687ab2 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -797,7 +797,7 @@ static void cmd_name(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_get_route(&bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Device is not available.\n");
+			perror("Device is not available.");
 			exit(1);
 		}
 	}
@@ -860,7 +860,7 @@ static void cmd_info(int dev_id, int argc, char **argv)
 		dev_id = hci_get_route(&bdaddr);
 
 	if (dev_id < 0) {
-		fprintf(stderr, "Device is not available or not connected.\n");
+		perror("Device is not available or not connected.");
 		exit(1);
 	}
 
@@ -1231,7 +1231,7 @@ static void cmd_cc(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_get_route(&bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Device is not available.\n");
+			perror("Device is not available.");
 			exit(1);
 		}
 	}
@@ -1288,7 +1288,7 @@ static void cmd_dc(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1369,7 +1369,7 @@ static void cmd_sr(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1426,7 +1426,7 @@ static void cmd_rssi(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1500,7 +1500,7 @@ static void cmd_lq(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1576,7 +1576,7 @@ static void cmd_tpl(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1652,7 +1652,7 @@ static void cmd_afh(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1739,7 +1739,7 @@ static void cmd_cpt(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1823,7 +1823,7 @@ static void cmd_lp(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -1861,13 +1861,13 @@ static void cmd_lp(int dev_id, int argc, char **argv)
 			printf("Link policy settings: %s\n", str);
 			bt_free(str);
 		} else {
-			fprintf(stderr, "Can't allocate memory\n");
+			perror("Can't allocate memory");
 			exit(1);
 		}
 	} else {
 		unsigned int val;
 		if (hci_strtolp(argv[1], &val) < 0) {
-			fprintf(stderr, "Invalig arguments\n");
+			perror("Invalig arguments");
 			exit(1);
 		}
 		policy = val;
@@ -1922,7 +1922,7 @@ static void cmd_lst(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -2012,7 +2012,7 @@ static void cmd_auth(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -2084,7 +2084,7 @@ static void cmd_enc(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -2157,7 +2157,7 @@ static void cmd_key(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
@@ -2229,7 +2229,7 @@ static void cmd_clkoff(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror( "Not connected.");
 			exit(1);
 		}
 	}
@@ -2306,7 +2306,7 @@ static void cmd_clock(int dev_id, int argc, char **argv)
 	if (dev_id < 0) {
 		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
 		if (dev_id < 0) {
-			fprintf(stderr, "Not connected.\n");
+			perror("Not connected.");
 			exit(1);
 		}
 	}
-- 
1.6.0.6

[PATCH 4/6] remove verification before free

[Gustavo F. Padovan]

When free(ptr), if ptr is NULL, no operation is performed by free, so we
don't need to test if ptr is NULL or not NULL.
---
 tools/hciconfig.c |    6 ++----
 tools/hcitool.c   |    3 +--
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index a89aed1..98e5536 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -950,10 +950,8 @@ static void cmd_version(int ctl, int hdev, char *opt)
 		lmpver ? lmpver : "n/a", ver.lmp_ver, ver.lmp_subver,
 		bt_compidtostr(ver.manufacturer), ver.manufacturer);
 
-	if (hciver)
-		bt_free(hciver);
-	if (lmpver)
-		bt_free(lmpver);
+	bt_free(hciver);
+	bt_free(lmpver);
 
 	hci_close_dev(dd);
 }
diff --git a/tools/hcitool.c b/tools/hcitool.c
index 9687ab2..8171c31 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -921,8 +921,7 @@ static void cmd_info(int dev_id, int argc, char **argv)
 			version.lmp_subver,
 			bt_compidtostr(version.manufacturer),
 			version.manufacturer);
-		if (ver)
-			bt_free(ver);
+		bt_free(ver);
 	}
 
 	memset(features, 0, sizeof(features));
-- 
1.6.0.6

[PATCH 5/6] Remove unnecessary attribution

[Gustavo F. Padovan]

---
 src/main.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/main.c b/src/main.c
index d5d45c0..c86ae96 100644
--- a/src/main.c
+++ b/src/main.c
@@ -220,7 +220,6 @@ static void update_service_classes(const bdaddr_t *bdaddr, uint8_t value)
 	dl = g_malloc0(HCI_MAX_DEV * sizeof(*dr) + sizeof(*dl));
 
 	dl->dev_num = HCI_MAX_DEV;
-	dr = dl->dev_req;
 
 	if (ioctl(sk, HCIGETDEVLIST, dl) < 0) {
 		close(sk);
-- 
1.6.0.6

[PATCH 6/6] Make expand_name a void function

[Gustavo F. Padovan]

---
 src/main.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/main.c b/src/main.c
index c86ae96..5ca1b27 100644
--- a/src/main.c
+++ b/src/main.c
@@ -266,13 +266,13 @@ static void update_service_classes(const bdaddr_t *bdaddr, uint8_t value)
  * Device name expansion
  *   %d - device id
  */
-static char *expand_name(char *dst, int size, char *str, int dev_id)
+static void expand_name(char *dst, int size, char *str, int dev_id)
 {
 	register int sp, np, olen;
 	char *opt, buf[10];
 
 	if (!str && !dst)
-		return NULL;
+		return;
 
 	sp = np = 0;
 	while (np < size - 1 && str[sp]) {
@@ -317,7 +317,6 @@ static char *expand_name(char *dst, int size, char *str, int dev_id)
 		}
 	}
 	dst[np] = '\0';
-	return dst;
 }
 
 static gboolean child_exit(GIOChannel *io, GIOCondition cond, void *user_data)
-- 
1.6.0.6

Re: [PATCH 3/6] Change fprintf(stderr,...) to perror()

[David Vrabel]

Gustavo F. Padovan wrote:
> 
> @@ -1922,7 +1922,7 @@ static void cmd_lst(int dev_id, int argc, char **argv)
>  	if (dev_id < 0) {
>  		dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
>  		if (dev_id < 0) {
> -			fprintf(stderr, "Not connected.\n");
> +			perror("Not connected.");
>  			exit(1);
>  		}
>  	}

You should only perror() if errno is valid for the error being reported.
 Otherwise you're going to get odd messages like:

Not connected.: Success

David
-- 
David Vrabel, Senior Software Engineer, Drivers
CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/

Re: [PATCH 3/6] Change fprintf(stderr,...) to perror()

[Gustavo F. Padovan]

On Mon, Feb 16, 2009 at 9:21 AM, David Vrabel <david.vrabel@csr.com> wrote:
> Gustavo F. Padovan wrote:
>>
>> @@ -1922,7 +1922,7 @@ static void cmd_lst(int dev_id, int argc, char **argv)
>>       if (dev_id < 0) {
>>               dev_id = hci_for_each_dev(HCI_UP, find_conn, (long) &bdaddr);
>>               if (dev_id < 0) {
>> -                     fprintf(stderr, "Not connected.\n");
>> +                     perror("Not connected.");
>>                       exit(1);
>>               }
>>       }
>
> You should only perror() if errno is valid for the error being reported.
>  Otherwise you're going to get odd messages like:
>
> Not connected.: Success

Ok. You are right.

>
> David
> --
> David Vrabel, Senior Software Engineer, Drivers
> CSR, Churchill House, Cambridge Business Park,  Tel: +44 (0)1223 692562
> Cowley Road, Cambridge, CB4 0WZ                 http://www.csr.com/
>



-- 
Gustavo F. Padovan

Computer Engineering Student
Institute of Computing - IC
University of Campinas - UNICAMP

email: gustavo@las.ic.unicamp.br
gtalk: gfpadovan@gmail.com
mobile: +55 19 81030803