2020-12-28 06:49:18

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v1 1/4] shared/mgmt: Add supports of parsing mgmt tlv list

Response from Read System Default Configuration is a list of mgmt_tlv,
which requires further processing to get the values of each parameters.

This adds APIs for parsing response into mgmt_tlv_list, retrieving
parameter from mgmt_tlv_list.

Reviewed-by: [email protected]
---

src/shared/mgmt.c | 38 ++++++++++++++++++++++++++++++++++++++
src/shared/mgmt.h | 6 ++++++
2 files changed, 44 insertions(+)

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index 9ea9974f5535..bdc4bdd9e66b 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -626,6 +626,44 @@ static void mgmt_tlv_to_buf(void *data, void *user_data)
*buf_ptr += entry_size;
}

+struct mgmt_tlv_list *mgmt_tlv_list_load_from_buf(const uint8_t *buf,
+ uint16_t len)
+{
+ struct mgmt_tlv_list *tlv_list;
+ const uint8_t *cur = buf;
+
+ if (!len || !buf)
+ return NULL;
+
+ tlv_list = mgmt_tlv_list_new();
+
+ while (cur < buf + len) {
+ struct mgmt_tlv *entry = cur;
+
+ cur += sizeof(*entry) + entry->length;
+ if (cur > buf + len)
+ goto failed;
+
+ if (!mgmt_tlv_add(tlv_list, entry->type, entry->length,
+ entry->value)) {
+ goto failed;
+ }
+ }
+
+ return tlv_list;
+failed:
+ mgmt_tlv_list_free(tlv_list);
+
+ return NULL;
+}
+
+void mgmt_tlv_list_foreach(struct mgmt_tlv_list *tlv_list,
+ mgmt_tlv_list_foreach_func_t callback,
+ void *user_data)
+{
+ queue_foreach(tlv_list->tlv_queue, callback, user_data);
+}
+
unsigned int mgmt_send_tlv(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
struct mgmt_tlv_list *tlv_list,
mgmt_request_func_t callback,
diff --git a/src/shared/mgmt.h b/src/shared/mgmt.h
index 319beb62f9eb..808bf4c7ff09 100644
--- a/src/shared/mgmt.h
+++ b/src/shared/mgmt.h
@@ -41,6 +41,12 @@ bool mgmt_tlv_add(struct mgmt_tlv_list *tlv_list, uint16_t type, uint8_t length,
#define mgmt_tlv_add_fixed(_list, _type, _value) \
mgmt_tlv_add(_list, _type, sizeof(*(_value)), _value)

+struct mgmt_tlv_list *mgmt_tlv_list_load_from_buf(const uint8_t *buf,
+ uint16_t len);
+typedef void (*mgmt_tlv_list_foreach_func_t)(void *data, void *user_data);
+void mgmt_tlv_list_foreach(struct mgmt_tlv_list *tlv_list,
+ mgmt_tlv_list_foreach_func_t callback,
+ void *user_data);
unsigned int mgmt_send_tlv(struct mgmt *mgmt, uint16_t opcode, uint16_t index,
struct mgmt_tlv_list *tlv_list,
mgmt_request_func_t callback,
--
2.29.2.729.g45daf8777d-goog


2020-12-28 06:49:38

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v1 4/4] shared/mgmt: Fix memory leak in mgmt_tlv_list

This patch freed the mgmt_tlv properly in mgmt_tlv_list_free.

Reviewed-by: [email protected]
Reviewed-by: [email protected]
---

src/shared/mgmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/mgmt.c b/src/shared/mgmt.c
index bdc4bdd9e66b..19b5466f9071 100644
--- a/src/shared/mgmt.c
+++ b/src/shared/mgmt.c
@@ -595,7 +595,7 @@ static void mgmt_tlv_free(struct mgmt_tlv *entry)

void mgmt_tlv_list_free(struct mgmt_tlv_list *tlv_list)
{
- queue_destroy(tlv_list->tlv_queue, NULL);
+ queue_destroy(tlv_list->tlv_queue, mgmt_tlv_free);
free(tlv_list);
}

--
2.29.2.729.g45daf8777d-goog

2020-12-28 06:50:18

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v1 2/4] btmgmt: Add command read-sysconfig

Add command read-sysconfig in btmgmt

Example usage:
localhost ~ # btmgmt
[mgmt]# read-sysconfig
Type: 0x0000 Length: 02 Value: 0000
Type: 0x0001 Length: 02 Value: 0008
...
Type: 0x001f Length: 01 Value: 01

Reviewed-by: [email protected]
---

tools/btmgmt.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)

diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index 2f7cb2efcc38..c766de862a12 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -1752,6 +1752,53 @@ static void cmd_exp_debug(int argc, char **argv)
}
}

+static void print_mgmt_tlv(void *data, void *user_data)
+{
+ const struct mgmt_tlv *entry = data;
+ char buf[256];
+
+ bin2hex(entry->value, entry->length, buf, sizeof(buf));
+ print("Type: 0x%04x\tLength: %02hhu\tValue: %s", entry->type,
+ entry->length, buf);
+}
+
+static void read_sysconfig_rsp(uint8_t status, uint16_t len, const void *param,
+ void *user_data)
+{
+ struct mgmt_tlv_list *tlv_list;
+ struct mgmt_tlv *entry;
+
+ if (status != 0) {
+ error("Read system configuration failed with status "
+ "0x%02x (%s)", status, mgmt_errstr(status));
+ return;
+ }
+
+ tlv_list = mgmt_tlv_list_load_from_buf(param, len);
+ if (!tlv_list) {
+ error("Unable to parse response of read system configuration");
+ return;
+ }
+
+ mgmt_tlv_list_foreach(tlv_list, print_mgmt_tlv, NULL);
+ mgmt_tlv_list_free(tlv_list);
+}
+
+static void cmd_read_sysconfig(int argc, char **argv)
+{
+ uint16_t index;
+
+ index = mgmt_index;
+ if (index == MGMT_INDEX_NONE)
+ index = 0;
+
+ if (!mgmt_send(mgmt, MGMT_OP_READ_DEF_SYSTEM_CONFIG, index,
+ 0, NULL, read_sysconfig_rsp, NULL, NULL)) {
+ error("Unable to send read system configuration cmd");
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }
+}
+
static void auto_power_enable_rsp(uint8_t status, uint16_t len,
const void *param, void *user_data)
{
@@ -5030,6 +5077,8 @@ static const struct bt_shell_menu main_menu = {
cmd_expinfo, "Show experimental features" },
{ "exp-debug", "<on/off>",
cmd_exp_debug, "Set debug feature" },
+ { "read-sysconfig", NULL,
+ cmd_read_sysconfig, "Read System Configuration" },
{} },
};

--
2.29.2.729.g45daf8777d-goog

2020-12-28 06:50:19

by Yun-hao Chung

[permalink] [raw]
Subject: [Bluez PATCH v1 3/4] btmgmt: Add command set-sysconfig

Add command set-sysconfig in btmgmt

Example usage:
[mgmt]# set-sysconfig -h
Parameters:
-v <type:length:value>...
e.g.:
set-sysconfig -v 001a:2:1234 001f:1:00
[mgmt]# set-sysconfig -v 8:2:abcd 1:02:0100 0016:2:0600

Reviewed-by: [email protected]
---

tools/btmgmt.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)

diff --git a/tools/btmgmt.c b/tools/btmgmt.c
index c766de862a12..4cc94efe3f1a 100644
--- a/tools/btmgmt.c
+++ b/tools/btmgmt.c
@@ -1799,6 +1799,113 @@ static void cmd_read_sysconfig(int argc, char **argv)
}
}

+static bool parse_mgmt_tlv(const char *input, uint16_t *type, uint8_t *length,
+ uint8_t *value)
+{
+ int i, value_starting_pos;
+
+ if (sscanf(input, "%4hx:%1hhu:%n", type, length,
+ &value_starting_pos) < 2) {
+ return false;
+ }
+
+ input += value_starting_pos;
+
+ if (*length * 2 != strlen(input))
+ return false;
+
+ for (i = 0; i < *length; i++) {
+ if (!sscanf(input + i * 2, "%2hhx", &value[i]))
+ return false;
+ }
+
+ return true;
+}
+
+static void set_sysconfig_rsp(uint8_t status, uint16_t len, const void *param,
+ void *user_data)
+{
+ if (status != MGMT_STATUS_SUCCESS) {
+ error("Could not set default system configuration with status "
+ "0x%02x (%s)", status, mgmt_errstr(status));
+ return bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }
+
+ print("Set default system configuration success");
+
+ return bt_shell_noninteractive_quit(EXIT_SUCCESS);
+}
+
+static bool set_sysconfig(int argc, char **argv)
+{
+ struct mgmt_tlv_list *tlv_list = NULL;
+ int i;
+ uint16_t index, type;
+ uint8_t length;
+ uint8_t value[256] = {};
+ bool success = false;
+
+ index = mgmt_index;
+ if (index == MGMT_INDEX_NONE)
+ index = 0;
+
+ tlv_list = mgmt_tlv_list_new();
+ if (!tlv_list) {
+ error("tlv_list failed to init");
+ goto failed;
+ }
+
+ for (i = 0; i < argc; i++) {
+ if (!parse_mgmt_tlv(argv[i], &type, &length, value)) {
+ error("failed to parse");
+ goto failed;
+ }
+
+ if (!mgmt_tlv_add(tlv_list, type, length, value)) {
+ error("failed to add");
+ goto failed;
+ }
+ }
+
+ if (!mgmt_send_tlv(mgmt, MGMT_OP_SET_DEF_SYSTEM_CONFIG, index,
+ tlv_list, set_sysconfig_rsp, NULL, NULL)) {
+ error("Failed to send \"Set Default System Configuration\""
+ " command");
+ goto failed;
+ }
+
+ success = true;
+
+failed:
+ if (tlv_list)
+ mgmt_tlv_list_free(tlv_list);
+
+ return success;
+}
+
+static void set_sysconfig_usage(void)
+{
+ bt_shell_usage();
+ print("Parameters:\n\t-v <type:length:value>...\n"
+ "e.g.:\n\tset-sysconfig -v 001a:2:1234 001f:1:00");
+}
+
+static void cmd_set_sysconfig(int argc, char **argv)
+{
+ bool success = false;
+
+ if (strcasecmp(argv[1], "-v") == 0 && argc > 2) {
+ argc -= 2;
+ argv += 2;
+ success = set_sysconfig(argc, argv);
+ }
+
+ if (!success) {
+ set_sysconfig_usage();
+ bt_shell_noninteractive_quit(EXIT_FAILURE);
+ }
+}
+
static void auto_power_enable_rsp(uint8_t status, uint16_t len,
const void *param, void *user_data)
{
@@ -5079,6 +5186,8 @@ static const struct bt_shell_menu main_menu = {
cmd_exp_debug, "Set debug feature" },
{ "read-sysconfig", NULL,
cmd_read_sysconfig, "Read System Configuration" },
+ { "set-sysconfig", "<-v|-h> [options...]",
+ cmd_set_sysconfig, "Set System Configuration" },
{} },
};

--
2.29.2.729.g45daf8777d-goog

2020-12-28 07:11:16

by bluez.test.bot

[permalink] [raw]
Subject: RE: [Bluez,v1,1/4] shared/mgmt: Add supports of parsing mgmt tlv list

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=406659

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
btmgmt: Add command set-sysconfig
WARNING:NAKED_SSCANF: unchecked sscanf return value
#42: FILE: tools/btmgmt.c:1818:
+ if (!sscanf(input + i * 2, "%2hhx", &value[i]))
+ return false;

- total: 0 errors, 1 warnings, 121 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] btmgmt: Add command set-sysconfig" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - FAIL
Output:
btmgmt: Add command set-sysconfig
8: B3 Line contains hard tab characters (\t): " -v <type:length:value>..."
10: B3 Line contains hard tab characters (\t): " set-sysconfig -v 001a:2:1234 001f:1:00"


##############################
Test: CheckBuild - FAIL
Output:
src/shared/mgmt.c: In function ‘mgmt_tlv_list_free’:
src/shared/mgmt.c:598:37: error: passing argument 2 of ‘queue_destroy’ from incompatible pointer type [-Werror=incompatible-pointer-types]
598 | queue_destroy(tlv_list->tlv_queue, mgmt_tlv_free);
| ^~~~~~~~~~~~~
| |
| void (*)(struct mgmt_tlv *)
In file included from src/shared/mgmt.c:25:
./src/shared/queue.h:23:62: note: expected ‘queue_destroy_func_t’ {aka ‘void (*)(void *)’} but argument is of type ‘void (*)(struct mgmt_tlv *)’
23 | void queue_destroy(struct queue *queue, queue_destroy_func_t destroy);
| ~~~~~~~~~~~~~~~~~~~~~^~~~~~~
src/shared/mgmt.c: In function ‘mgmt_tlv_list_load_from_buf’:
src/shared/mgmt.c:641:28: error: initialization of ‘struct mgmt_tlv *’ from incompatible pointer type ‘const uint8_t *’ {aka ‘const unsigned char *’} [-Werror=incompatible-pointer-types]
641 | struct mgmt_tlv *entry = cur;
| ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:6808: src/shared/mgmt.lo] Error 1
make: *** [Makefile:4023: all] Error 2


##############################
Test: MakeCheck - SKIPPED
Output:
checkbuild not success



---
Regards,
Linux Bluetooth