tools: Name must be "hciX", where X is 0..HCI_MAX_DEV. Any other like "tty1, ""hci001"
or "hci1x" will no longer work as "hci1".
tools: Add info that "bdaddr" can be put for parameter "interface".
---
attrib/gatttool.c | 9 ++++++---
compat/pand.c | 2 +-
lib/hci.c | 53 +++++++++++++++++++++++++++++++++++++++++++++--------
lib/hci_lib.h | 1 +
test/hciemu.c | 9 +++++----
test/l2test.c | 8 +++++---
test/rctest.c | 10 ++++++----
tools/ciptool.c | 9 +++++----
tools/hciconfig.c | 8 +++++++-
tools/hcitool.c | 4 ++--
tools/l2ping.c | 12 +++++++-----
tools/main.c | 7 ++++---
tools/sdptool.c | 9 +++++----
13 files changed, 99 insertions(+), 42 deletions(-)
diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index a234e36..43b4f57 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -105,10 +105,13 @@ static GIOChannel *do_connect(gboolean le)
/* Local adapter */
if (opt_src != NULL) {
- if (!strncmp(opt_src, "hci", 3))
- hci_devba(atoi(opt_src + 3), &sba);
- else
+ int devid;
+
+ devid = hci_filter_devname(opt_src);
+ if (devid < 0)
str2ba(opt_src, &sba);
+ else
+ hci_devba(devid, &sba);
} else
bacpy(&sba, BDADDR_ANY);
diff --git a/compat/pand.c b/compat/pand.c
index c3860fa..7331fad 100644
--- a/compat/pand.c
+++ b/compat/pand.c
@@ -586,7 +586,7 @@ static const char *main_help =
"\t--role -r <role> Local PAN role (PANU, NAP, GN)\n"
"\t--service -d <role> Remote PAN service (PANU, NAP, GN)\n"
"\t--ethernet -e <name> Network interface name\n"
- "\t--device -i <bdaddr> Source bdaddr\n"
+ "\t--device -i <hciX|bdaddr> Source interface or bdaddr\n"
"\t--nosdp -D Disable SDP\n"
"\t--auth -A Enable authentication\n"
"\t--encrypt -E Enable encryption\n"
diff --git a/lib/hci.c b/lib/hci.c
index 048fda4..bb01d90 100644
--- a/lib/hci.c
+++ b/lib/hci.c
@@ -887,22 +887,57 @@ int hci_get_route(bdaddr_t *bdaddr)
(long) (bdaddr ? bdaddr : BDADDR_ANY));
}
+/* return 1 if string is ecimal number without leading zeros
+ * return 0 if not */
+static int is_devnumber(const char *c)
+{
+ if (c[0] == '0' && c[1] != 0)
+ return 0;
+
+ while (*c) {
+ if (*c < '0' || *c > '9')
+ return 0;
+ ++c;
+ }
+ return 1;
+}
+
+/* return HCI dev_id from string like "hciX", where X is dev_id
+ * return -1 if string not match */
+int hci_filter_devname(const char *str)
+{
+ int dev_id;
+
+ if ((strlen(str) >= 4) && (!strncmp(str, "hci", 3)) &&
+ (is_devnumber(str + 3)))
+ dev_id = atoi(str + 3);
+ else
+ dev_id = -1;
+
+ if (dev_id >= HCI_MAX_DEV)
+ dev_id = -1;
+
+ return dev_id;
+}
+
+/* return dev_id, which is on UP state, from 'hciX' or 'bdaddr'
+ * otherwise return -1 */
int hci_devid(const char *str)
{
bdaddr_t ba;
- int id = -1;
+ int dev_id;
- if (!strncmp(str, "hci", 3) && strlen(str) >= 4) {
- id = atoi(str + 3);
- if (hci_devba(id, &ba) < 0)
- return -1;
- } else {
+ dev_id = hci_filter_devname(str);
+ if (dev_id < 0) {
errno = ENODEV;
str2ba(str, &ba);
- id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
+ dev_id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
+ } else {
+ if (hci_devba(dev_id, &ba) < 0)
+ return -1;
}
- return id;
+ return dev_id;
}
int hci_devinfo(int dev_id, struct hci_dev_info *di)
@@ -925,6 +960,8 @@ int hci_devinfo(int dev_id, struct hci_dev_info *di)
return ret;
}
+/* return status 0 and bdaddr assigned to dev_id
+ * otherwise status -1 */
int hci_devba(int dev_id, bdaddr_t *bdaddr)
{
struct hci_dev_info di;
diff --git a/lib/hci_lib.h b/lib/hci_lib.h
index b63a2a4..ef00f99 100644
--- a/lib/hci_lib.h
+++ b/lib/hci_lib.h
@@ -60,6 +60,7 @@ int hci_inquiry(int dev_id, int len, int num_rsp, const uint8_t *lap, inquiry_in
int hci_devinfo(int dev_id, struct hci_dev_info *di);
int hci_devba(int dev_id, bdaddr_t *bdaddr);
int hci_devid(const char *str);
+int hci_filter_devname(const char *str);
int hci_read_local_name(int dd, int len, char *name, int to);
int hci_write_local_name(int dd, const char *name, int to);
diff --git a/test/hciemu.c b/test/hciemu.c
index a20374f..ae33d72 100644
--- a/test/hciemu.c
+++ b/test/hciemu.c
@@ -1234,15 +1234,16 @@ int main(int argc, char *argv[])
exit(1);
}
- if (strlen(argv[0]) > 3 && !strncasecmp(argv[0], "hci", 3)) {
+ dev = hci_filter_devname(argv[0]);
+ if (dev < 0) {
+ if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
+ exit(1);
+ } else {
dev = hci_devid(argv[0]);
if (dev < 0) {
perror("Invalid device");
exit(1);
}
- } else {
- if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
- exit(1);
}
if (detach) {
diff --git a/test/l2test.c b/test/l2test.c
index 17883a9..438ba39 100644
--- a/test/l2test.c
+++ b/test/l2test.c
@@ -1101,6 +1101,7 @@ int main(int argc, char *argv[])
{
struct sigaction sa;
int opt, sk, mode = RECV, need_addr = 0;
+ int devid;
bacpy(&bdaddr, BDADDR_ANY);
@@ -1175,10 +1176,11 @@ int main(int argc, char *argv[])
break;
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'P':
diff --git a/test/rctest.c b/test/rctest.c
index b3804f5..a828ad9 100644
--- a/test/rctest.c
+++ b/test/rctest.c
@@ -579,7 +579,7 @@ static void usage(void)
"\t-m multiple connects\n");
printf("Options:\n"
- "\t[-b bytes] [-i device] [-P channel] [-U uuid]\n"
+ "\t[-b bytes] [-i hciX|bdaddr] [-P channel] [-U uuid]\n"
"\t[-L seconds] enabled SO_LINGER option\n"
"\t[-W seconds] enable deferred setup\n"
"\t[-B filename] use data packets from file\n"
@@ -597,6 +597,7 @@ int main(int argc, char *argv[])
{
struct sigaction sa;
int opt, sk, mode = RECV, need_addr = 0;
+ int devid;
bacpy(&bdaddr, BDADDR_ANY);
@@ -644,10 +645,11 @@ int main(int argc, char *argv[])
break;
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'P':
diff --git a/tools/ciptool.c b/tools/ciptool.c
index edce9da..ec602ef 100644
--- a/tools/ciptool.c
+++ b/tools/ciptool.c
@@ -427,7 +427,7 @@ static void usage(void)
"\n");
printf("Options:\n"
- "\t-i [hciX|bdaddr] Local HCI device or BD Address\n"
+ "\t-i <hciX|bdaddr> Local HCI device or BD Address\n"
"\t-h, --help Display help\n"
"\n");
@@ -455,10 +455,11 @@ int main(int argc, char *argv[])
while ((opt = getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (!strncmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ i = hci_filter_devname(optarg);
+ if (i < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(i, &bdaddr);
break;
case 'h':
usage();
diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index f0ae11c..e20963d 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -1849,7 +1849,13 @@ int main(int argc, char *argv[])
exit(0);
}
- di.dev_id = atoi(argv[0] + 3);
+ i = hci_filter_devname(argv[0]);
+ if (i < 0) {
+ fprintf(stderr, "No valid device name.\n");
+ exit(1);
+ }
+ di.dev_id = i;
+
argc--; argv++;
if (ioctl(ctl, HCIGETDEVINFO, (void *) &di)) {
diff --git a/tools/hcitool.c b/tools/hcitool.c
index d50adaf..dc70e63 100644
--- a/tools/hcitool.c
+++ b/tools/hcitool.c
@@ -2560,8 +2560,8 @@ static void usage(void)
printf("Usage:\n"
"\thcitool [options] <command> [command parameters]\n");
printf("Options:\n"
- "\t--help\tDisplay help\n"
- "\t-i dev\tHCI device\n");
+ "\t--help Display help\n"
+ "\t-i <hciX|bdaddr> Local HCI device or BD Address\n");
printf("Commands:\n");
for (i = 0; command[i].cmd; i++)
printf("\t%-4s\t%s\n", command[i].cmd,
diff --git a/tools/l2ping.c b/tools/l2ping.c
index 29fb3d0..dd0ccbd 100644
--- a/tools/l2ping.c
+++ b/tools/l2ping.c
@@ -255,7 +255,8 @@ static void usage(void)
{
printf("l2ping - L2CAP ping\n");
printf("Usage:\n");
- printf("\tl2ping [-i device] [-s size] [-c count] [-t timeout] [-d delay] [-f] [-r] [-v] <bdaddr>\n");
+ printf("\tl2ping [-i <hciX|bdaddr> local hci or bd address] [-s size]"
+ "[-c count] [-t timeout] [-d delay] [-f] [-r] [-v] <bdaddr>\n");
printf("\t-f Flood ping (delay = 0)\n");
printf("\t-r Reverse ping\n");
printf("\t-v Verify request and response payload\n");
@@ -264,17 +265,18 @@ static void usage(void)
int main(int argc, char *argv[])
{
int opt;
-
+ int devid;
/* Default options */
bacpy(&bdaddr, BDADDR_ANY);
while ((opt=getopt(argc,argv,"i:d:s:c:t:frv")) != EOF) {
switch(opt) {
case 'i':
- if (!strncasecmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ devid = hci_filter_devname(optarg);
+ if (devid < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(devid, &bdaddr);
break;
case 'd':
diff --git a/tools/main.c b/tools/main.c
index 6800445..c000fba 100644
--- a/tools/main.c
+++ b/tools/main.c
@@ -753,10 +753,11 @@ int main(int argc, char *argv[])
while ((opt = getopt_long(argc, argv, "+i:f:rahAESML:", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (strncmp(optarg, "hci", 3) == 0)
- hci_devba(atoi(optarg + 3), &bdaddr);
- else
+ dev_id = hci_filter_devname(optarg);
+ if (dev_id < 0)
str2ba(optarg, &bdaddr);
+ else
+ hci_devba(dev_id, &bdaddr);
break;
case 'f':
diff --git a/tools/sdptool.c b/tools/sdptool.c
index 140a46a..c782e62 100644
--- a/tools/sdptool.c
+++ b/tools/sdptool.c
@@ -4209,7 +4209,7 @@ static void usage(void)
"\tsdptool [options] <command> [command parameters]\n");
printf("Options:\n"
"\t-h\t\tDisplay help\n"
- "\t-i\t\tSpecify source interface\n");
+ "\t-i\t\tSpecify source interface or bdaddr\n");
printf("Commands:\n");
for (i = 0; command[i].cmd; i++)
@@ -4242,10 +4242,11 @@ int main(int argc, char *argv[])
while ((opt=getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
switch(opt) {
case 'i':
- if (!strncmp(optarg, "hci", 3))
- hci_devba(atoi(optarg + 3), &interface);
- else
+ i = hci_filter_devname(optarg);
+ if (i < 0)
str2ba(optarg, &interface);
+ else
+ hci_devba(i, &interface);
break;
case 'h':
--
1.7.0.4
>Hi,
>
>This patch would not be necessary if you keep only str2ba() and
>ba2str() (like I commented on patch 2/4).
>
>Regards,
Cannot keep only *2*() functions, because for two reason. Firstly, these functions are different (see discussion at patch 2/4).
Secondly, for example, "ba2str" use external buffer, while "batostr" create it internal. So simpler is using "batostr",
because there is no need to create temporary buffer.
So I think patch is (quite) useful.
Regards
in lib/bluetooth.c make "str2ba" using "strtoba", "ba2str" using "batostr"
add comments describe differences between these functions
---
lib/bluetooth.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 4af2ef6..3a4e3f4 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -55,8 +55,7 @@ char *batostr(const bdaddr_t *ba)
return NULL;
sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- ba->b[0], ba->b[1], ba->b[2],
- ba->b[3], ba->b[4], ba->b[5]);
+ ba->b[0], ba->b[1], ba->b[2], ba->b[3], ba->b[4], ba->b[5]);
return str;
}
@@ -80,29 +79,30 @@ bdaddr_t *strtoba(const char *str)
return (bdaddr_t *) ba;
}
+/* reverse bdaddr and do batostr */
int ba2str(const bdaddr_t *ba, char *str)
{
- uint8_t b[6];
-
- baswap((bdaddr_t *) b, ba);
- return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- b[0], b[1], b[2], b[3], b[4], b[5]);
+ bdaddr_t b;
+ char *bastr;
+
+ baswap(&b, ba);
+ bastr = batostr(&b);
+ strcpy(str, bastr);
+ bt_free(bastr);
+ return *str && 1;
}
+/* do strtoba and return reverse bdaddr */
int str2ba(const char *str, bdaddr_t *ba)
{
- uint8_t b[6];
- const char *ptr = str;
+ bdaddr_t *b;
int i;
- for (i = 0; i < 6; i++) {
- b[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr, ':')))
- ptr = ":00:00:00:00:00";
- ptr++;
- }
-
- baswap(ba, (bdaddr_t *) b);
+ b = strtoba(str);
+ if (b == NULL)
+ return 0;
+ baswap(ba, b);
+ bt_free(b);
return 0;
}
--
1.7.0.4
---
lib/bluetooth.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 3a4e3f4..4e32a1d 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -60,23 +60,63 @@ char *batostr(const bdaddr_t *ba)
return str;
}
+/* convert to bdaddr_t string describes with regular expression "(XX:){0,5}XX"
+ * where X is "[0-9A-Fa-f]"
+ * shorter address is filling with ":00" at the end
+ */
bdaddr_t *strtoba(const char *str)
{
const char *ptr = str;
int i;
+ int parts;
+ int length;
+ bdaddr_t *ba;
+
+ length = strlen(str);
+ if (length <= 1 || length > 17)
+ return NULL;
- uint8_t *ba = bt_malloc(sizeof(bdaddr_t));
+ ba = bt_malloc(sizeof(bdaddr_t));
if (!ba)
return NULL;
- for (i = 0; i < 6; i++) {
- ba[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr,':')))
- ptr = ":00:00:00:00:00";
+ i = 0;
+ parts = 0;
+ while (*ptr) {
+ if (i == 2) {
+ if (*ptr == ':') {
+ i = 0;
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ parts++;
+ ptr++;
+ } else {
+ bt_free(ba);
+ return NULL;
+ }
+ }
+
+ if ((*ptr < 'A' || *ptr > 'F') &&
+ (*ptr < 'a' || *ptr > 'f') &&
+ (*ptr < '0' || *ptr > '9')) {
+ bt_free(ba);
+ return NULL;
+ }
+
+ i++;
ptr++;
}
- return (bdaddr_t *) ba;
+ if (*(ptr - 1) == ':' || i == 1) {
+ bt_free(ba);
+ return NULL;
+ }
+
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ for (i = parts + 1 ; i < 6 ; i++) {
+ ba->b[i] = 0;
+ }
+
+ return ba;
}
/* reverse bdaddr and do batostr */
--
1.7.0.4
On Mon, Dec 27, 2010 at 10:39 AM, Anderson Lizardo
<[email protected]> wrote:
> Hi,
>
> On Mon, Dec 27, 2010 at 10:35 AM, ?<[email protected]> wrote:
>> On Mon, Dec 27, 2010, Anderson Lizardo wrote:
>>> IMHO, you should keep str2ba and drop the few occurrences of
>>> strtoba(). Same applies to ba2str/batostr.
>>
>>> strtoba() allocates memory by itself, so if you use it instead, you
>>> need to deallocate memory. str2ba(), on the other hand, uses a buffer
>>> given as argument.
>>
>> These function are similar, but not the same. ba2str/str2ba reverse bdaddr, but
>> batostr/strtoba not. I wrote relevant documentation comments, because names look
>> similar, code look similar, but results are different.
>
> Ok, I confess the function names are confusing. One has to look at
> their implementation to make sure which one to use...
Just one more comment: on your commit description, it may appear that
you are merging the functions, while in fact you are just factoring
out common code. I suggest rewording the message so it makes clear you
are factoring out code.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
Hi,
On Mon, Dec 27, 2010 at 10:35 AM, <[email protected]> wrote:
> On Mon, Dec 27, 2010, Anderson Lizardo wrote:
>> IMHO, you should keep str2ba and drop the few occurrences of
>> strtoba(). Same applies to ba2str/batostr.
>
>> strtoba() allocates memory by itself, so if you use it instead, you
>> need to deallocate memory. str2ba(), on the other hand, uses a buffer
>> given as argument.
>
> These function are similar, but not the same. ba2str/str2ba reverse bdaddr, but
> batostr/strtoba not. I wrote relevant documentation comments, because names look
> similar, code look similar, but results are different.
Ok, I confess the function names are confusing. One has to look at
their implementation to make sure which one to use...
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
On Mon, Dec 27, 2010, Anderson Lizardo wrote:
> IMHO, you should keep str2ba and drop the few occurrences of
> strtoba(). Same applies to ba2str/batostr.
> strtoba() allocates memory by itself, so if you use it instead, you
> need to deallocate memory. str2ba(), on the other hand, uses a buffer
> given as argument.
These function are similar, but not the same. ba2str/str2ba reverse bdaddr, but
batostr/strtoba not. I wrote relevant documentation comments, because names look
similar, code look similar, but results are different.
Regards
Hi,
On Mon, Dec 27, 2010 at 7:02 AM, Michal Labedzki
<[email protected]> wrote:
> +/* return 1 if string is ecimal number without leading zeros
> + * return 0 if not */
Typo: ecimal -> decimal
> +static int is_devnumber(const char *c)
> +{
> + ? ? ? if (c[0] == '0' && c[1] != 0)
> + ? ? ? ? ? ? ? return 0;
> +
> + ? ? ? while (*c) {
> + ? ? ? ? ? ? ? if (*c < '0' || *c > '9')
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> + ? ? ? ? ? ? ? ++c;
> + ? ? ? }
> + ? ? ? return 1;
> +}
> +
> +/* return HCI dev_id from string like "hciX", where X is dev_id
> + * return -1 if string not match */
Suggestion: "return -1 if str has invalid format"
> +int hci_filter_devname(const char *str)
> +{
> + ? ? ? int dev_id;
> +
> + ? ? ? if ((strlen(str) >= 4) && (!strncmp(str, "hci", 3)) &&
> + ? ? ? ? ? ? ? (is_devnumber(str + 3)))
> + ? ? ? ? ? ? ? dev_id = atoi(str + 3);
> + ? ? ? else
> + ? ? ? ? ? ? ? dev_id = -1;
> +
> + ? ? ? if (dev_id >= HCI_MAX_DEV)
> + ? ? ? ? ? ? ? dev_id = -1;
> +
> + ? ? ? return dev_id;
> +}
> +
> +/* return dev_id, which is on UP state, from 'hciX' or 'bdaddr'
> + * otherwise return -1 */
> ?int hci_devid(const char *str)
> ?{
> ? ? ? ?bdaddr_t ba;
> - ? ? ? int id = -1;
> + ? ? ? int dev_id;
>
> - ? ? ? if (!strncmp(str, "hci", 3) && strlen(str) >= 4) {
> - ? ? ? ? ? ? ? id = atoi(str + 3);
> - ? ? ? ? ? ? ? if (hci_devba(id, &ba) < 0)
> - ? ? ? ? ? ? ? ? ? ? ? return -1;
> - ? ? ? } else {
> + ? ? ? dev_id = hci_filter_devname(str);
> + ? ? ? if (dev_id < 0) {
> ? ? ? ? ? ? ? ?errno = ENODEV;
> ? ? ? ? ? ? ? ?str2ba(str, &ba);
> - ? ? ? ? ? ? ? id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
> + ? ? ? ? ? ? ? dev_id = hci_for_each_dev(HCI_UP, __same_bdaddr, (long) &ba);
> + ? ? ? } else {
> + ? ? ? ? ? ? ? if (hci_devba(dev_id, &ba) < 0)
> + ? ? ? ? ? ? ? ? ? ? ? return -1;
> ? ? ? ?}
Use "else if (hci_devba(...))" and drop the extra brackets.
>
> - ? ? ? return id;
> + ? ? ? return dev_id;
> ?}
> diff --git a/test/hciemu.c b/test/hciemu.c
> index a20374f..ae33d72 100644
> --- a/test/hciemu.c
> +++ b/test/hciemu.c
> @@ -1234,15 +1234,16 @@ int main(int argc, char *argv[])
> ? ? ? ? ? ? ? ?exit(1);
> ? ? ? ?}
>
> - ? ? ? if (strlen(argv[0]) > 3 && !strncasecmp(argv[0], "hci", 3)) {
> + ? ? ? dev = hci_filter_devname(argv[0]);
> + ? ? ? if (dev < 0) {
> + ? ? ? ? ? ? ? if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
> + ? ? ? ? ? ? ? ? ? ? ? exit(1);
> + ? ? ? } else {
> ? ? ? ? ? ? ? ?dev = hci_devid(argv[0]);
> ? ? ? ? ? ? ? ?if (dev < 0) {
> ? ? ? ? ? ? ? ? ? ? ? ?perror("Invalid device");
> ? ? ? ? ? ? ? ? ? ? ? ?exit(1);
> ? ? ? ? ? ? ? ?}
> - ? ? ? } else {
> - ? ? ? ? ? ? ? if (getbdaddrbyname(argv[0], &vdev.bdaddr) < 0)
> - ? ? ? ? ? ? ? ? ? ? ? exit(1);
> ? ? ? ?}
>
> ? ? ? ?if (detach) {
> diff --git a/test/l2test.c b/test/l2test.c
> index 17883a9..438ba39 100644
> --- a/test/l2test.c
> +++ b/test/l2test.c
> @@ -1101,6 +1101,7 @@ int main(int argc, char *argv[])
> ?{
> ? ? ? ?struct sigaction sa;
> ? ? ? ?int opt, sk, mode = RECV, need_addr = 0;
> + ? ? ? int devid;
>
> ? ? ? ?bacpy(&bdaddr, BDADDR_ANY);
>
> @@ -1175,10 +1176,11 @@ int main(int argc, char *argv[])
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (!strncasecmp(optarg, "hci", 3))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &bdaddr);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? devid = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (devid < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &bdaddr);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(devid, &bdaddr);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'P':
> diff --git a/test/rctest.c b/test/rctest.c
> index b3804f5..a828ad9 100644
> --- a/test/rctest.c
> +++ b/test/rctest.c
> @@ -579,7 +579,7 @@ static void usage(void)
> ? ? ? ? ? ? ? ?"\t-m multiple connects\n");
>
> ? ? ? ?printf("Options:\n"
> - ? ? ? ? ? ? ? "\t[-b bytes] [-i device] [-P channel] [-U uuid]\n"
> + ? ? ? ? ? ? ? "\t[-b bytes] [-i hciX|bdaddr] [-P channel] [-U uuid]\n"
> ? ? ? ? ? ? ? ?"\t[-L seconds] enabled SO_LINGER option\n"
> ? ? ? ? ? ? ? ?"\t[-W seconds] enable deferred setup\n"
> ? ? ? ? ? ? ? ?"\t[-B filename] use data packets from file\n"
> @@ -597,6 +597,7 @@ int main(int argc, char *argv[])
> ?{
> ? ? ? ?struct sigaction sa;
> ? ? ? ?int opt, sk, mode = RECV, need_addr = 0;
> + ? ? ? int devid;
>
> ? ? ? ?bacpy(&bdaddr, BDADDR_ANY);
>
> @@ -644,10 +645,11 @@ int main(int argc, char *argv[])
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (!strncasecmp(optarg, "hci", 3))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &bdaddr);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? devid = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (devid < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &bdaddr);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(devid, &bdaddr);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'P':
> diff --git a/tools/ciptool.c b/tools/ciptool.c
> index edce9da..ec602ef 100644
> --- a/tools/ciptool.c
> +++ b/tools/ciptool.c
> @@ -427,7 +427,7 @@ static void usage(void)
> ? ? ? ? ? ? ? ?"\n");
>
> ? ? ? ?printf("Options:\n"
> - ? ? ? ? ? ? ? "\t-i [hciX|bdaddr] ? Local HCI device or BD Address\n"
> + ? ? ? ? ? ? ? "\t-i <hciX|bdaddr> ? Local HCI device or BD Address\n"
> ? ? ? ? ? ? ? ?"\t-h, --help ? ? ? ? Display help\n"
> ? ? ? ? ? ? ? ?"\n");
>
> @@ -455,10 +455,11 @@ int main(int argc, char *argv[])
> ? ? ? ?while ((opt = getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
> ? ? ? ? ? ? ? ?switch(opt) {
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (!strncmp(optarg, "hci", 3))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &bdaddr);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? i = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (i < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &bdaddr);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(i, &bdaddr);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case 'h':
> ? ? ? ? ? ? ? ? ? ? ? ?usage();
> diff --git a/tools/hciconfig.c b/tools/hciconfig.c
> index f0ae11c..e20963d 100644
> --- a/tools/hciconfig.c
> +++ b/tools/hciconfig.c
> @@ -1849,7 +1849,13 @@ int main(int argc, char *argv[])
> ? ? ? ? ? ? ? ?exit(0);
> ? ? ? ?}
>
> - ? ? ? di.dev_id = atoi(argv[0] + 3);
> + ? ? ? i = hci_filter_devname(argv[0]);
> + ? ? ? if (i < 0) {
> + ? ? ? ? ? ? ? fprintf(stderr, "No valid device name.\n");
> + ? ? ? ? ? ? ? exit(1);
> + ? ? ? }
> + ? ? ? di.dev_id = i;
> +
> ? ? ? ?argc--; argv++;
>
> ? ? ? ?if (ioctl(ctl, HCIGETDEVINFO, (void *) &di)) {
> diff --git a/tools/hcitool.c b/tools/hcitool.c
> index d50adaf..dc70e63 100644
> --- a/tools/hcitool.c
> +++ b/tools/hcitool.c
> @@ -2560,8 +2560,8 @@ static void usage(void)
> ? ? ? ?printf("Usage:\n"
> ? ? ? ? ? ? ? ?"\thcitool [options] <command> [command parameters]\n");
> ? ? ? ?printf("Options:\n"
> - ? ? ? ? ? ? ? "\t--help\tDisplay help\n"
> - ? ? ? ? ? ? ? "\t-i dev\tHCI device\n");
> + ? ? ? ? ? ? ? "\t--help ? ? ? ? ? ? ? ? Display help\n"
> + ? ? ? ? ? ? ? "\t-i <hciX|bdaddr> ? ? ? Local HCI device or BD Address\n");
> ? ? ? ?printf("Commands:\n");
> ? ? ? ?for (i = 0; command[i].cmd; i++)
> ? ? ? ? ? ? ? ?printf("\t%-4s\t%s\n", command[i].cmd,
> diff --git a/tools/l2ping.c b/tools/l2ping.c
> index 29fb3d0..dd0ccbd 100644
> --- a/tools/l2ping.c
> +++ b/tools/l2ping.c
> @@ -255,7 +255,8 @@ static void usage(void)
> ?{
> ? ? ? ?printf("l2ping - L2CAP ping\n");
> ? ? ? ?printf("Usage:\n");
> - ? ? ? printf("\tl2ping [-i device] [-s size] [-c count] [-t timeout] [-d delay] [-f] [-r] [-v] <bdaddr>\n");
> + ? ? ? printf("\tl2ping [-i <hciX|bdaddr> local hci or bd address] [-s size]"
> + ? ? ? ? ? ? ? "[-c count] [-t timeout] [-d delay] [-f] [-r] [-v] <bdaddr>\n");
The change above looks strange. You could drop "local hci or bd
address" (it is implicit from "<hciX|bdaddr>").
> ? ? ? ?printf("\t-f ?Flood ping (delay = 0)\n");
> ? ? ? ?printf("\t-r ?Reverse ping\n");
> ? ? ? ?printf("\t-v ?Verify request and response payload\n");
> @@ -264,17 +265,18 @@ static void usage(void)
> ?int main(int argc, char *argv[])
> ?{
> ? ? ? ?int opt;
> -
> + ? ? ? int devid;
Add an empty line here.
> ? ? ? ?/* Default options */
> ? ? ? ?bacpy(&bdaddr, BDADDR_ANY);
>
> ? ? ? ?while ((opt=getopt(argc,argv,"i:d:s:c:t:frv")) != EOF) {
> ? ? ? ? ? ? ? ?switch(opt) {
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (!strncasecmp(optarg, "hci", 3))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &bdaddr);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? devid = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (devid < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &bdaddr);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(devid, &bdaddr);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'd':
> diff --git a/tools/main.c b/tools/main.c
> index 6800445..c000fba 100644
> --- a/tools/main.c
> +++ b/tools/main.c
> @@ -753,10 +753,11 @@ int main(int argc, char *argv[])
> ? ? ? ?while ((opt = getopt_long(argc, argv, "+i:f:rahAESML:", main_options, NULL)) != -1) {
> ? ? ? ? ? ? ? ?switch(opt) {
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (strncmp(optarg, "hci", 3) == 0)
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &bdaddr);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? dev_id = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (dev_id < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &bdaddr);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(dev_id, &bdaddr);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'f':
> diff --git a/tools/sdptool.c b/tools/sdptool.c
> index 140a46a..c782e62 100644
> --- a/tools/sdptool.c
> +++ b/tools/sdptool.c
> @@ -4209,7 +4209,7 @@ static void usage(void)
> ? ? ? ? ? ? ? ?"\tsdptool [options] <command> [command parameters]\n");
> ? ? ? ?printf("Options:\n"
> ? ? ? ? ? ? ? ?"\t-h\t\tDisplay help\n"
> - ? ? ? ? ? ? ? "\t-i\t\tSpecify source interface\n");
> + ? ? ? ? ? ? ? "\t-i\t\tSpecify source interface or bdaddr\n");
>
> ? ? ? ?printf("Commands:\n");
> ? ? ? ?for (i = 0; command[i].cmd; i++)
> @@ -4242,10 +4242,11 @@ int main(int argc, char *argv[])
> ? ? ? ?while ((opt=getopt_long(argc, argv, "+i:h", main_options, NULL)) != -1) {
> ? ? ? ? ? ? ? ?switch(opt) {
> ? ? ? ? ? ? ? ?case 'i':
> - ? ? ? ? ? ? ? ? ? ? ? if (!strncmp(optarg, "hci", 3))
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(atoi(optarg + 3), &interface);
> - ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? i = hci_filter_devname(optarg);
> + ? ? ? ? ? ? ? ? ? ? ? if (i < 0)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?str2ba(optarg, &interface);
> + ? ? ? ? ? ? ? ? ? ? ? else
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hci_devba(i, &interface);
> ? ? ? ? ? ? ? ? ? ? ? ?break;
>
> ? ? ? ? ? ? ? ?case 'h':
> --
> 1.7.0.4
>
> --
> 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
>
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
Hi,
This patch would not be necessary if you keep only str2ba() and
ba2str() (like I commented on patch 2/4).
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
On Mon, Dec 27, 2010 at 7:02 AM, Michal Labedzki
<[email protected]> wrote:
> + ? ? ? ? ? ? ? if (((*ptr < 'A' || *ptr > 'F') &&
> + ? ? ? ? ? ? ? ? ? ? ? (*ptr < 'a' || *ptr > 'f') &&
> + ? ? ? ? ? ? ? ? ? ? ? (*ptr < '0' || *ptr > '9')) ||
> + ? ? ? ? ? ? ? ? ? ? ? ( (ptr - str) == 2 && *ptr != ':')) {
> + ? ? ? ? ? ? ? ? ? ? ? bt_free(ba);
> + ? ? ? ? ? ? ? ? ? ? ? goto err_bdaddr;
> + ? ? ? ? ? ? ? }
The logic above seems wrong. Make sure you are not mixing || and &&.
> +
> + ? ? ? ? ? ? ? ++i;
> + ? ? ? ? ? ? ? ++ptr;
I think the usual coding style in bluez uses post-increment in these
cases (e.g. i++).
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
On Mon, Dec 27, 2010 at 7:02 AM, Michal Labedzki
<[email protected]> wrote:
> in lib/bluetooth.c merge "str2ba" and "strtoba", "ba2str" and "batostr"
IMHO, you should keep str2ba and drop the few occurrences of
strtoba(). Same applies to ba2str/batostr.
strtoba() allocates memory by itself, so if you use it instead, you
need to deallocate memory. str2ba(), on the other hand, uses a buffer
given as argument.
Regards,
--
Anderson Lizardo
OpenBossa Labs - INdT
Manaus - Brazil
---
lib/bluetooth.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 3a4e3f4..9333eeb 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -60,23 +60,64 @@ char *batostr(const bdaddr_t *ba)
return str;
}
+/* convert to bdaddr_t string describes with regular expression "(XX:){0,5}XX"
+ * where X is "[0-9A-Fa-f]"
+ * shorter address is filling with ":00" at the end
+ */
bdaddr_t *strtoba(const char *str)
{
const char *ptr = str;
int i;
+ int parts;
+ int length;
+ bdaddr_t *ba;
+ unsigned int b;
+
+ length = strlen(str);
+ if (length <= 1 || length > 17)
+ goto err_bdaddr;
- uint8_t *ba = bt_malloc(sizeof(bdaddr_t));
+ ba = bt_malloc(sizeof(bdaddr_t));
if (!ba)
return NULL;
- for (i = 0; i < 6; i++) {
- ba[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr,':')))
- ptr = ":00:00:00:00:00";
- ptr++;
+ i = 0;
+ parts = 0;
+ while (*ptr) {
+ if (i == 2 && *ptr == ':') {
+ i = 0;
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ ++parts;
+ ++ptr;
+ }
+
+ if (((*ptr < 'A' || *ptr > 'F') &&
+ (*ptr < 'a' || *ptr > 'f') &&
+ (*ptr < '0' || *ptr > '9')) ||
+ ( (ptr - str) == 2 && *ptr != ':')) {
+ bt_free(ba);
+ goto err_bdaddr;
+ }
+
+ ++i;
+ ++ptr;
}
- return (bdaddr_t *) ba;
+ if (*(ptr - 1) == ':' || i == 1) {
+ bt_free(ba);
+ goto err_bdaddr;
+ }
+
+ ba->b[parts] = strtol(ptr - 2, NULL, 16);
+ for (i = parts + 1 ; i < 6 ; ++i) {
+ ba->b[i] = 0;
+ }
+
+ return ba;
+
+err_bdaddr:
+ fprintf(stderr, "Incorrect bdaddr format\n");
+ exit(1);
}
/* reverse bdaddr and do batostr */
--
1.7.0.4
---
compat/bnep.c | 5 +++--
compat/pand.c | 9 ++++++---
test/hciemu.c | 8 ++++++--
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/compat/bnep.c b/compat/bnep.c
index 9b0d8b8..f651dfd 100644
--- a/compat/bnep.c
+++ b/compat/bnep.c
@@ -137,9 +137,10 @@ int bnep_show_connections(void)
}
for (i = 0; i < req.cnum; i++) {
- printf("%s %s %s\n", ci[i].device,
- batostr((bdaddr_t *) ci[i].dst),
+ char *tmp_bastr = batostr((bdaddr_t *) ci[i].dst);
+ printf("%s %s %s\n", ci[i].device, tmp_bastr,
bnep_svc2str(ci[i].role));
+ bt_free(tmp_bastr);
}
return 0;
}
diff --git a/compat/pand.c b/compat/pand.c
index 7331fad..8a50020 100644
--- a/compat/pand.c
+++ b/compat/pand.c
@@ -456,10 +456,13 @@ static void do_show(void)
static void do_kill(char *dst)
{
- if (dst)
- bnep_kill_connection((void *) strtoba(dst));
- else
+ if (dst) {
+ bdaddr_t *tmp_ba = strtoba(dst);
+ bnep_kill_connection((void *) tmp_ba);
+ bt_free(tmp_ba);
+ } else {
bnep_kill_all_connections();
+ }
}
static void sig_hup(int sig)
diff --git a/test/hciemu.c b/test/hciemu.c
index ae33d72..031b867 100644
--- a/test/hciemu.c
+++ b/test/hciemu.c
@@ -499,8 +499,10 @@ static void accept_connection(uint8_t *data)
static void close_connection(struct vhci_conn *conn)
{
+ char *tmp_bastr = batostr(&conn->dest);
syslog(LOG_INFO, "Closing connection %s handle %d",
- batostr(&conn->dest), conn->handle);
+ tmp_bastr, conn->handle);
+ bt_free(tmp_bastr);
g_io_channel_close(conn->chan);
g_io_channel_unref(conn->chan);
@@ -1017,7 +1019,9 @@ static int getbdaddrbyname(char *str, bdaddr_t *ba)
if (n == 5) {
/* BD address */
- baswap(ba, strtoba(str));
+ bdaddr_t *tmp_ba = strtoba(str);
+ baswap(ba, tmp_ba);
+ bt_free(tmp_ba);
return 0;
}
--
1.7.0.4
in lib/bluetooth.c merge "str2ba" and "strtoba", "ba2str" and "batostr"
---
lib/bluetooth.c | 34 +++++++++++++++++-----------------
1 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 4af2ef6..3a4e3f4 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -55,8 +55,7 @@ char *batostr(const bdaddr_t *ba)
return NULL;
sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- ba->b[0], ba->b[1], ba->b[2],
- ba->b[3], ba->b[4], ba->b[5]);
+ ba->b[0], ba->b[1], ba->b[2], ba->b[3], ba->b[4], ba->b[5]);
return str;
}
@@ -80,29 +79,30 @@ bdaddr_t *strtoba(const char *str)
return (bdaddr_t *) ba;
}
+/* reverse bdaddr and do batostr */
int ba2str(const bdaddr_t *ba, char *str)
{
- uint8_t b[6];
-
- baswap((bdaddr_t *) b, ba);
- return sprintf(str, "%2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X",
- b[0], b[1], b[2], b[3], b[4], b[5]);
+ bdaddr_t b;
+ char *bastr;
+
+ baswap(&b, ba);
+ bastr = batostr(&b);
+ strcpy(str, bastr);
+ bt_free(bastr);
+ return *str && 1;
}
+/* do strtoba and return reverse bdaddr */
int str2ba(const char *str, bdaddr_t *ba)
{
- uint8_t b[6];
- const char *ptr = str;
+ bdaddr_t *b;
int i;
- for (i = 0; i < 6; i++) {
- b[i] = (uint8_t) strtol(ptr, NULL, 16);
- if (i != 5 && !(ptr = strchr(ptr, ':')))
- ptr = ":00:00:00:00:00";
- ptr++;
- }
-
- baswap(ba, (bdaddr_t *) b);
+ b = strtoba(str);
+ if (b == NULL)
+ return 0;
+ baswap(ba, b);
+ bt_free(b);
return 0;
}
--
1.7.0.4