2017-11-26 20:21:23

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/4] shared/shell: Detect if the arguments are properly set

From: Luiz Augusto von Dentz <[email protected]>

Parse arguments to detect if user has passed a valid number of
arguments.
---
src/shared/shell.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 92 insertions(+), 6 deletions(-)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index c271a53ee..73aedaf79 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -207,6 +207,95 @@ static void shell_print_menu(void)
}
}

+static int parse_args(char *arg, wordexp_t *w, char *del, int flags)
+{
+ char *str;
+
+ str = g_strdelimit(arg, del, '"');
+
+ if (wordexp(str, w, flags)) {
+ g_free(str);
+ return -EINVAL;
+ }
+
+ /* If argument ends with ,,, set we_offs bypass strict checks */
+ if (w->we_wordc && g_str_has_suffix(w->we_wordv[w->we_wordc -1], "..."))
+ w->we_offs = 1;
+
+ g_free(str);
+
+ return 0;
+}
+
+static int cmd_exec(const struct bt_shell_menu_entry *entry,
+ int argc, char *argv[])
+{
+ wordexp_t w;
+ size_t len;
+ char *man, *opt;
+ int flags = WRDE_NOCMD;
+
+ if (!entry->arg || entry->arg[0] == '\0') {
+ if (argc) {
+ print_text(COLOR_HIGHLIGHT, "Too many arguments");
+ return -EINVAL;
+ }
+ goto exec;
+ }
+
+ /* Find last mandatory arguments */
+ man = strrchr(entry->arg, '>');
+ if (!man) {
+ opt = strdup(entry->arg);
+ goto optional;
+ }
+
+ len = man - entry->arg;
+ man = strndup(entry->arg, len + 1);
+
+ if (parse_args(man, &w, "<>", flags) < 0) {
+ print_text(COLOR_HIGHLIGHT,
+ "Unable to parse mandatory command arguments");
+ return -EINVAL;
+ }
+
+ /* Check if there are enough arguments */
+ if ((unsigned) argc < w.we_wordc && !w.we_offs) {
+ print_text(COLOR_HIGHLIGHT, "Missing %s argument",
+ w.we_wordv[argc]);
+ goto fail;
+ }
+
+ flags |= WRDE_APPEND;
+ opt = strdup(entry->arg + len + 1);
+
+optional:
+ if (parse_args(opt, &w, "<>", flags) < 0) {
+ print_text(COLOR_HIGHLIGHT,
+ "Unable to parse mandatory command arguments");
+ return -EINVAL;
+ }
+
+ /* Check if there are too many arguments */
+ if ((unsigned) argc > w.we_wordc && !w.we_offs) {
+ print_text(COLOR_HIGHLIGHT, "Too many arguments: %d > %zu",
+ argc, w.we_wordc);
+ goto fail;
+ }
+
+ wordfree(&w);
+
+exec:
+ if (entry->func)
+ entry->func(argc, argv);
+
+ return 0;
+
+fail:
+ wordfree(&w);
+ return -EINVAL;
+}
+
static int menu_exec(const struct bt_shell_menu_entry *entry,
int argc, char *argv[])
{
@@ -222,10 +311,7 @@ static int menu_exec(const struct bt_shell_menu_entry *entry,
if (data.menu == data.main && !strcmp(entry->cmd, "back"))
continue;

- if (entry->func) {
- entry->func(argc - 1, ++argv);
- return 0;
- }
+ return cmd_exec(entry, --argc, ++argv);
}

return -ENOENT;
@@ -236,8 +322,8 @@ static void shell_exec(int argc, char *argv[])
if (!data.menu || !argv[0])
return;

- if (menu_exec(default_menu, argc, argv) < 0) {
- if (menu_exec(data.menu->entries, argc, argv) < 0)
+ if (menu_exec(default_menu, argc, argv) == -ENOENT) {
+ if (menu_exec(data.menu->entries, argc, argv) == -ENOENT)
print_text(COLOR_HIGHLIGHT, "Invalid command");
}
}
--
2.13.6



2017-11-27 04:17:10

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH BlueZ 4/4] shared/shell: Add default argument generator

Hi Luiz,

On 11/27/2017 05:21 AM, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This adds a default argument generator if the command don't provide
> one, the generator will print out the argument list so the user don't
> have to always resort to help for checking it.

The generator complements the unusable string in the command input line.
So we have to delete it for inputting a right value/string if the argument
list is complemented.

I think that showing the usage/argument outside the command input line is
better than complementing the argument list.


Regards,
Eramoto


2017-11-26 20:21:26

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 4/4] shared/shell: Add default argument generator

From: Luiz Augusto von Dentz <[email protected]>

This adds a default argument generator if the command don't provide
one, the generator will print out the argument list so the user don't
have to always resort to help for checking it.
---
src/shared/shell.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/shared/shell.c b/src/shared/shell.c
index 73aedaf79..d0cd2e6fe 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -518,6 +518,17 @@ static char *cmd_generator(const char *text, int state)
return find_cmd(text, data.menu->entries, &index);
}

+static char *arg_generator(const char *text, int state)
+{
+ if (!text)
+ return NULL;
+
+ if (!state)
+ return strdup(text);
+
+ return NULL;
+}
+
static char **menu_completion(const struct bt_shell_menu_entry *entry,
const char *text, char *input_cmd)
{
@@ -527,8 +538,12 @@ static char **menu_completion(const struct bt_shell_menu_entry *entry,
if (strcmp(entry->cmd, input_cmd))
continue;

- if (!entry->gen)
- continue;
+ if (!entry->gen) {
+ rl_completion_display_matches_hook = NULL;
+ matches = rl_completion_matches(entry->arg,
+ arg_generator);
+ break;
+ }

rl_completion_display_matches_hook = entry->disp;
matches = rl_completion_matches(text, entry->gen);
--
2.13.6


2017-11-26 20:21:25

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/4] client: Fix format of arguments of set-service and set-manufacturer

From: Luiz Augusto von Dentz <[email protected]>

Optional arguments should contain only one set of '[' ']' otherwise
wordexp won't be able to parse it.
---
client/main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/client/main.c b/client/main.c
index 38028c6c0..7661b33c0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2205,9 +2205,9 @@ static const struct bt_shell_menu advertise_menu = {
.entries = {
{ "set-uuids", "[uuid1 uuid2 ...]",
cmd_set_advertise_uuids, "Set advertise uuids" },
- { "set-service", "[uuid][data=[xx xx ...]", cmd_set_advertise_service,
+ { "set-service", "[uuid] [data=xx xx ...]", cmd_set_advertise_service,
"Set advertise service data" },
- { "set-manufacturer", "[id][data=[xx xx ...]",
+ { "set-manufacturer", "[id] [data=xx xx ...]",
cmd_set_advertise_manufacturer,
"Set advertise manufacturer data" },
{ "set-tx-power", "<on/off>", cmd_set_advertise_tx_power,
@@ -2254,7 +2254,7 @@ static const struct bt_shell_menu gatt_menu = {
{ "attribute-info", "[attribute/UUID]", cmd_attribute_info,
"Select attribute", attribute_generator },
{ "read", NULL, cmd_read, "Read attribute value" },
- { "write", "<data=[xx xx ...]>", cmd_write,
+ { "write", "<data=xx xx ...>", cmd_write,
"Write attribute value" },
{ "acquire-write", NULL, cmd_acquire_write,
"Acquire Write file descriptor" },
--
2.13.6


2017-11-26 20:21:24

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/4] client: Remove argument checks

From: Luiz Augusto von Dentz <[email protected]>

This is now done automatically by bt_shell with the use of the
arguments entered in the command table.
---
client/main.c | 93 -----------------------------------------------------------
1 file changed, 93 deletions(-)

diff --git a/client/main.c b/client/main.c
index a510851e3..38028c6c0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -804,14 +804,6 @@ static gboolean parse_argument(int argc, char *argv[], const char **arg_table,
{
const char **opt;

- if (!argc || !strlen(argv[0])) {
- if (msg)
- bt_shell_printf("Missing on/off/%s argument\n", msg);
- else
- bt_shell_printf("Missing on/off argument\n");
- return FALSE;
- }
-
if (!strcmp(argv[0], "on") || !strcmp(argv[0], "yes")) {
*value = TRUE;
if (option)
@@ -889,11 +881,6 @@ static void cmd_select(int argc, char *argv[])
{
struct adapter *adapter;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing controller address argument\n");
- return;
- }
-
adapter = find_ctrl_by_address(ctrl_list, argv[0]);
if (!adapter) {
bt_shell_printf("Controller %s not available\n", argv[0]);
@@ -959,11 +946,6 @@ static void cmd_system_alias(int argc, char *argv[])
{
char *name;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing name argument\n");
- return;
- }
-
if (check_default_ctrl() == FALSE)
return;

@@ -1637,11 +1619,6 @@ static void cmd_remove(int argc, char *argv[])
{
GDBusProxy *proxy;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing device address argument\n");
- return;
- }
-
if (check_default_ctrl() == FALSE)
return;

@@ -1688,11 +1665,6 @@ static void cmd_connect(int argc, char *argv[])
{
GDBusProxy *proxy;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing device address argument\n");
- return;
- }
-
if (check_default_ctrl() == FALSE)
return;

@@ -1773,11 +1745,6 @@ static void cmd_set_alias(int argc, char *argv[])
{
char *name;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing name argument\n");
- return;
- }
-
if (!default_dev) {
bt_shell_printf("No device connected\n");
return;
@@ -1797,11 +1764,6 @@ static void cmd_select_attribute(int argc, char *argv[])
{
GDBusProxy *proxy;

- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing attribute argument\n");
- return;
- }
-
if (!default_dev) {
bt_shell_printf("No device connected\n");
return;
@@ -1889,11 +1851,6 @@ static void cmd_read(int argc, char *argv[])

static void cmd_write(int argc, char *argv[])
{
- if (!argc || !strlen(argv[0])) {
- bt_shell_printf("Missing data argument\n");
- return;
- }
-
if (!default_attr) {
bt_shell_printf("No attribute selected\n");
return;
@@ -1978,11 +1935,6 @@ static void cmd_register_service(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (!argc) {
- bt_shell_printf("Missing argument\n");
- return;
- }
-
gatt_register_service(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -1991,11 +1943,6 @@ static void cmd_unregister_service(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (!argc) {
- bt_shell_printf("Missing argument\n");
- return;
- }
-
gatt_unregister_service(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -2004,11 +1951,6 @@ static void cmd_register_characteristic(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (argc < 2) {
- bt_shell_printf("Missing arguments\n");
- return;
- }
-
gatt_register_chrc(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -2017,11 +1959,6 @@ static void cmd_unregister_characteristic(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (argc < 1) {
- bt_shell_printf("Missing arguments\n");
- return;
- }
-
gatt_unregister_chrc(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -2030,11 +1967,6 @@ static void cmd_register_descriptor(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (argc < 2) {
- bt_shell_printf("Missing arguments\n");
- return;
- }
-
gatt_register_desc(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -2043,11 +1975,6 @@ static void cmd_unregister_descriptor(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return;

- if (argc < 1) {
- bt_shell_printf("Missing arguments\n");
- return;
- }
-
gatt_unregister_desc(dbus_conn, default_ctrl->proxy, argc, argv);
}

@@ -2208,11 +2135,6 @@ static void cmd_set_advertise_tx_power(int argc, char *argv[])

static void cmd_set_advertise_name(int argc, char *argv[])
{
- if (!argc || strlen(argv[0]) == 0) {
- bt_shell_printf("Missing on/off argument\n");
- return;
- }
-
if (strcmp(argv[0], "on") == 0 || strcmp(argv[0], "yes") == 0) {
ad_advertise_name(dbus_conn, true);
return;
@@ -2231,11 +2153,6 @@ static void cmd_set_advertise_appearance(int argc, char *argv[])
long int value;
char *endptr = NULL;

- if (!argc || strlen(argv[0]) == 0) {
- bt_shell_printf("Missing value argument\n");
- return;
- }
-
if (strcmp(argv[0], "on") == 0 || strcmp(argv[0], "yes") == 0) {
ad_advertise_appearance(dbus_conn, true);
return;
@@ -2260,11 +2177,6 @@ static void cmd_set_advertise_duration(int argc, char *argv[])
long int value;
char *endptr = NULL;

- if (!argc || strlen(argv[0]) == 0) {
- bt_shell_printf("Missing value argument\n");
- return;
- }
-
value = strtol(argv[0], &endptr, 0);
if (!endptr || *endptr != '\0' || value > UINT16_MAX) {
bt_shell_printf("Invalid argument\n");
@@ -2279,11 +2191,6 @@ static void cmd_set_advertise_timeout(int argc, char *argv[])
long int value;
char *endptr = NULL;

- if (!argc || strlen(argv[0]) == 0) {
- bt_shell_printf("Missing value argument\n");
- return;
- }
-
value = strtol(argv[0], &endptr, 0);
if (!endptr || *endptr != '\0' || value > UINT16_MAX) {
bt_shell_printf("Invalid argument\n");
--
2.13.6