[PATCH v2 0/7] topology: Add support for vendor tuples

[PATCH v2 0/7] topology: Add support for vendor tuples

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

This series addes support for vendor tuples to topology, to avoid
importing binary data blob from other files.

Backward compatibility of ABI is not impacted. A kernel patch is also
submitted "ASoC: topology: ABI - Define types for vendor tuples".

The 1st patch is small code cleanup.
The 2nd patch is a preparation, since tuples will need the type-specific
free handler.

History:
v2: add check on string length, use strtol() to get hex value,
    and fix memory leak.

Mengdong Lin (7):
  topology: Use the generic pointer to free an element's object
  topology: Define a free handler for the element
  topology: Add doc for vendor tuples
  topology: ABI - Define types for vendor tuples
  topology: Add support for vendor tokens
  topology: Add support for parsing vendor tuples
  topology: Build data objects with tuples

 include/sound/asoc.h      |  42 +++-
 include/topology.h        |  79 +++++++-
 src/topology/data.c       | 496 +++++++++++++++++++++++++++++++++++++++++++++-
 src/topology/elem.c       |  15 +-
 src/topology/parser.c     |  24 +++
 src/topology/tplg_local.h |  47 +++++
 6 files changed, 695 insertions(+), 8 deletions(-)

-- 
2.5.0

[PATCH v2 1/7] topology: Use the generic pointer to free an element's object

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

The element is a wrapper for different types of objects.So use the
generic pointer 'obj' instead of the type-specific pointer to free
the object.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/elem.c b/src/topology/elem.c
index 12d6a72..00f9eea 100644
--- a/src/topology/elem.c
+++ b/src/topology/elem.c
@@ -83,8 +83,8 @@ void tplg_elem_free(struct tplg_elem *elem)
 	/* free struct snd_tplg_ object,
 	 * the union pointers share the same address
 	 */
-	if (elem->mixer_ctrl)
-		free(elem->mixer_ctrl);
+	if (elem->obj)
+		free(elem->obj);
 
 	free(elem);
 }
-- 
2.5.0

[PATCH v2 2/7] topology: Define a free handler for the element

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

This handler is defined for type-specific destruction of an element.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/elem.c b/src/topology/elem.c
index 00f9eea..f2afaaf 100644
--- a/src/topology/elem.c
+++ b/src/topology/elem.c
@@ -83,8 +83,12 @@ void tplg_elem_free(struct tplg_elem *elem)
 	/* free struct snd_tplg_ object,
 	 * the union pointers share the same address
 	 */
-	if (elem->obj)
+	if (elem->obj) {
+		if (elem->free)
+			elem->free(elem->obj);
+
 		free(elem->obj);
+	}
 
 	free(elem);
 }
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 4915b1a..7368a86 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -127,6 +127,8 @@ struct tplg_elem {
 	 */
 	struct list_head ref_list;
 	struct list_head list; /* list of all elements with same type */
+
+	void (*free)(void *obj);
 };
 
 struct map_elem {
-- 
2.5.0

[PATCH v2 3/7] topology: Add doc for vendor tuples

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Describe how to define vendor tokens and tuples in the text conf file.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 011f6ae..51d282f 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -203,12 +203,77 @@ extern "C" {
  *	bytes "0x12,0x34,0x56,0x78"
  *	shorts "0x1122,0x3344,0x5566,0x7788"
  *	words "0xaabbccdd,0x11223344,0x66aa77bb,0xefef1234"
+ *	tuples "section id of the vendor tuples"
  * };
  * </pre>
- * The file, bytes, shorts and words keywords are all mutually exclusive as
- * the private data should only be taken from one source.  The private data can
- * either be read from a separate file or defined in the topology file using
- * the bytes, shorts or words keywords.
+ * The file, bytes, shorts, words and tuples keywords are all mutually
+ * exclusive as the private data should only be taken from one source.
+ * The private data can either be read from a separate file or defined in
+ * the topology file using the bytes, shorts, words or tuples keywords.
+ * The keyword tuples is to define vendor specific tuples. Please refer to
+ * section Vendor Tokens and Vendor tuples.
+ *
+ *  <h6>Vendor Tokens</h6>
+ * A vendor token list is defined as a new section. Each token element is
+ * a pair of string ID and integer value. And both the ID and value are
+ * vendor-specific.
+ *
+ * <pre>
+ * SectionVendorTokens."id of the vendor tokens" {
+ *	comment "optional comments"
+ *	VENDOR_TOKEN_ID1 "1"
+ *	VENDOR_TOKEN_ID2 "2"
+ *	VENDOR_TOKEN_ID3 "3"
+ *	...
+ * }
+ * </pre>
+ *
+ *  <h6>Vendor Tuples</h6>
+ * Vendor tuples are defined as a new section. It contains a reference to
+ * a vendor token list and several tuple arrays.
+ * All arrays share a vendor token list, defined by the tokens keyword.
+ * Each tuple array is for a specific type, defined by the string following
+ * the tuples keyword. Supported types are: string, uuid, bool, byte,
+ * short and word.
+ *
+ * <pre>
+ * SectionVendorTuples."id of the vendor tuples" {
+ *	tokens "id of the vendor tokens"
+ *
+ *	tuples."string" {
+ *		VENDOR_TOKEN_ID1 "character string"
+ *		...
+ *	}
+ *
+ *	tuples."uuid" {
+ *		VENDOR_TOKEN_ID2 "16 character uuid"
+ *		...
+ *	}
+ *
+ *	tuples."bool" {
+ *		VENDOR_TOKEN_ID3 "true/false"
+ *		...
+ *	}
+ *
+ *	tuples."byte" {
+ *		VENDOR_TOKEN_ID4 "0x11"
+ *		VENDOR_TOKEN_ID5 "0x22"
+ *		...
+ *	}
+ *
+ *	tuples."short" {
+ *		VENDOR_TOKEN_ID6 "0x1122"
+ *		VENDOR_TOKEN_ID7 "0x3344"
+ *		...
+ *	}
+ *
+ *	tuples."word" {
+ *		VENDOR_TOKEN_ID8 "0x11223344"
+ *		VENDOR_TOKEN_ID9 "0x55667788"
+ *		...
+ *	}
+ * }
+ * </pre>
  *
  * <h5>Mixer Controls</h5>
  * A mixer control is defined as a new section that can include channel mapping,
@@ -389,6 +454,10 @@ extern "C" {
  * fields are the same for widgets as they are for controls whilst the other
  * fields map on very closely to the driver widget fields.
  *
+ * <h5>Widget Private Data</h5>
+ * Widget can have private data. For the format of the private data, please
+ * refer to section Control Private Data.
+ *
  * <h4>PCM Capabilities</h4>
  * Topology can also define the capabilities of FE and BE PCMs. Capabilities
  * can be defined with the following section :-
-- 
2.5.0

[PATCH v2 4/7] topology: ABI - Define types for vendor tuples

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Tuples, a pair of token and value, can be used to define vendor specific
data, for controls and widgets. This can avoid importing binary data blob
from other files.

Vendor specific tuple arrays will be embeded in the private data buffer
of a control or widget object. To be backward compatible, union is used
to define the tuple arrays in the existing private data ABI object
'struct snd_soc_tplg_private'.

Vendors need to make sure the token values defined by the topology conf
file match those defined by their driver.

Now supported tuple types are uuid, string, bool, byte, short and word.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/sound/asoc.h b/include/sound/asoc.h
index a29c05c..920c9e0 100644
--- a/include/sound/asoc.h
+++ b/include/sound/asoc.h
@@ -107,6 +107,14 @@
 #define SND_SOC_TPLG_STREAM_PLAYBACK	0
 #define SND_SOC_TPLG_STREAM_CAPTURE	1
 
+/* vendor tuple types */
+#define SND_SOC_TPLG_TUPLE_TYPE_UUID	0
+#define SND_SOC_TPLG_TUPLE_TYPE_STRING	1
+#define SND_SOC_TPLG_TUPLE_TYPE_BOOL	2
+#define SND_SOC_TPLG_TUPLE_TYPE_BYTE	3
+#define SND_SOC_TPLG_TUPLE_TYPE_WORD	4
+#define SND_SOC_TPLG_TUPLE_TYPE_SHORT	5
+
 /*
  * Block Header.
  * This header precedes all object and object arrays below.
@@ -123,6 +131,35 @@ struct snd_soc_tplg_hdr {
 	__le32 count;		/* number of elements in block */
 } __attribute__((packed));
 
+/* vendor tuple for uuid */
+struct snd_soc_tplg_vendor_uuid_elem {
+	__le32 token;
+	char uuid[16];
+} __attribute__((packed));
+
+/* vendor tuple for a bool/byte/short/word value */
+struct snd_soc_tplg_vendor_value_elem {
+	__le32 token;
+	__le32 value;
+} __attribute__((packed));
+
+/* vendor tuple for string */
+struct snd_soc_tplg_vendor_string_elem {
+	__le32 token;
+	char string[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+} __attribute__((packed));
+
+struct snd_soc_tplg_vendor_array {
+	__le32 size;	/* size in bytes of the array, including all elements */
+	__le32 type;	/* SND_SOC_TPLG_TUPLE_TYPE_ */
+	__le32 num_elems;	/* number of elements in array */
+	union {
+		struct snd_soc_tplg_vendor_uuid_elem uuid[0];
+		struct snd_soc_tplg_vendor_value_elem value[0];
+		struct snd_soc_tplg_vendor_string_elem string[0];
+	};
+} __attribute__((packed));
+
 /*
  * Private data.
  * All topology objects may have private data that can be used by the driver or
@@ -130,7 +167,10 @@ struct snd_soc_tplg_hdr {
  */
 struct snd_soc_tplg_private {
 	__le32 size;	/* in bytes of private data */
-	char data[0];
+	union {
+		char data[0];
+		struct snd_soc_tplg_vendor_array array[0];
+	};
 } __attribute__((packed));
 
 /*
-- 
2.5.0

[PATCH v2 5/7] topology: Add support for vendor tokens

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Vendor can define a token list in SectionVendorTokens. Each token element
is a pair of string ID and integer value. And both the ID and value are
vendor-specific.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 51d282f..0df112c 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -578,6 +578,7 @@ enum snd_tplg_type {
 	SND_TPLG_TYPE_BE,		/*!< BE DAI link */
 	SND_TPLG_TYPE_CC,		/*!< Hostless codec <-> codec link */
 	SND_TPLG_TYPE_MANIFEST,		/*!< Topology manifest */
+	SND_TPLG_TYPE_TOKEN,		/*!< Vendor tokens */
 };
 
 /**
diff --git a/src/topology/data.c b/src/topology/data.c
index 370c0fa..8455c15 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -253,6 +253,56 @@ static int tplg_parse_data_hex(snd_config_t *cfg, struct tplg_elem *elem,
 	return ret;
 }
 
+/* Parse vendor tokens
+ */
+int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED)
+{
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *id, *value;
+	struct tplg_elem *elem;
+	struct tplg_vendor_tokens *tokens;
+	int num_tokens = 0;
+
+	elem = tplg_elem_new_common(tplg, cfg, NULL, SND_TPLG_TYPE_TOKEN);
+	if (!elem)
+		return -ENOMEM;
+
+	snd_config_for_each(i, next, cfg) {
+		num_tokens++;
+	}
+
+	if (!num_tokens)
+		return 0;
+
+	tplg_dbg(" Vendor tokens: %s, %d tokens\n", elem->id, num_tokens);
+
+	tokens = calloc(1, sizeof(*tokens)
+			+ num_tokens * sizeof(struct tplg_token));
+	if (!tokens)
+		return -ENOMEM;
+	elem->tokens = tokens;
+
+	snd_config_for_each(i, next, cfg) {
+
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_id(n, &id) < 0)
+			continue;
+
+		if (snd_config_get_string(n, &value) < 0)
+			continue;
+
+		elem_copy_text(tokens->token[tokens->num_tokens].id, id,
+				SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+		tokens->token[tokens->num_tokens].value = atoi(value);
+		tplg_dbg("\t\t %s : %d\n", tokens->token[tokens->num_tokens].id,
+			tokens->token[tokens->num_tokens].value);
+		tokens->num_tokens++;
+	}
+
+	return 0;
+}
 
 /* Parse Private data.
  *
diff --git a/src/topology/elem.c b/src/topology/elem.c
index f2afaaf..95e3fd4 100644
--- a/src/topology/elem.c
+++ b/src/topology/elem.c
@@ -193,6 +193,9 @@ struct tplg_elem* tplg_elem_new_common(snd_tplg_t *tplg,
 		list_add_tail(&elem->list, &tplg->cc_list);
 		obj_size = sizeof(struct snd_soc_tplg_link_config);
 		break;
+	case SND_TPLG_TYPE_TOKEN:
+		list_add_tail(&elem->list, &tplg->token_list);
+		break;
 	default:
 		free(elem);
 		return NULL;
diff --git a/src/topology/parser.c b/src/topology/parser.c
index 4546257..264abc8 100644
--- a/src/topology/parser.c
+++ b/src/topology/parser.c
@@ -173,6 +173,14 @@ static int tplg_parse_config(snd_tplg_t *tplg, snd_config_t *cfg)
 			continue;
 		}
 
+		if (strcmp(id, "SectionVendorTokens") == 0) {
+			err = tplg_parse_compound(tplg, n, tplg_parse_tokens,
+				NULL);
+			if (err < 0)
+				return err;
+			continue;
+		}
+
 		SNDERR("error: unknown section %s\n", id);
 	}
 	return 0;
@@ -407,6 +415,7 @@ snd_tplg_t *snd_tplg_new(void)
 	INIT_LIST_HEAD(&tplg->mixer_list);
 	INIT_LIST_HEAD(&tplg->enum_list);
 	INIT_LIST_HEAD(&tplg->bytes_ext_list);
+	INIT_LIST_HEAD(&tplg->token_list);
 
 	return tplg;
 }
@@ -426,6 +435,7 @@ void snd_tplg_free(snd_tplg_t *tplg)
 	tplg_elem_free_list(&tplg->mixer_list);
 	tplg_elem_free_list(&tplg->enum_list);
 	tplg_elem_free_list(&tplg->bytes_ext_list);
+	tplg_elem_free_list(&tplg->token_list);
 
 	free(tplg);
 }
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 7368a86..679bfff 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -69,6 +69,7 @@ struct snd_tplg {
 	struct list_head route_list;
 	struct list_head text_list;
 	struct list_head pdata_list;
+	struct list_head token_list;
 	struct list_head pcm_config_list;
 	struct list_head pcm_caps_list;
 
@@ -86,6 +87,16 @@ struct tplg_ref {
 	struct list_head list;
 };
 
+/* element for vendor tokens */
+struct tplg_token {
+	char id[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	unsigned int value;
+};
+
+struct tplg_vendor_tokens {
+	unsigned int num_tokens;
+	struct tplg_token token[0];
+};
 /* topology element */
 struct tplg_elem {
 
@@ -118,6 +129,7 @@ struct tplg_elem {
 		/* these do not map to UAPI structs but are internal only */
 		struct snd_soc_tplg_ctl_tlv *tlv;
 		struct snd_soc_tplg_private *data;
+		struct tplg_vendor_tokens *tokens;
 	};
 
 	/* an element may refer to other elements:
@@ -151,6 +163,9 @@ int tplg_parse_text(snd_tplg_t *tplg, snd_config_t *cfg,
 int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 	void *private ATTRIBUTE_UNUSED);
 
+int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED);
+
 int tplg_parse_control_bytes(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
-- 
2.5.0

[PATCH v2 6/7] topology: Add support for parsing vendor tuples

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Vendor can define several tuple arrays in 'SectionVendorTuples', as
well as the reference to a vendor token list object.

A later patche will copy vendor tuples in ABI format to the private
buffer of its parent data object in the building phase.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/include/topology.h b/include/topology.h
index 0df112c..b47f422 100644
--- a/include/topology.h
+++ b/include/topology.h
@@ -579,6 +579,7 @@ enum snd_tplg_type {
 	SND_TPLG_TYPE_CC,		/*!< Hostless codec <-> codec link */
 	SND_TPLG_TYPE_MANIFEST,		/*!< Topology manifest */
 	SND_TPLG_TYPE_TOKEN,		/*!< Vendor tokens */
+	SND_TPLG_TYPE_TUPLE,		/*!< Vendor tuples */
 };
 
 /**
diff --git a/src/topology/data.c b/src/topology/data.c
index 8455c15..8ce1a0a 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -253,6 +253,168 @@ static int tplg_parse_data_hex(snd_config_t *cfg, struct tplg_elem *elem,
 	return ret;
 }
 
+static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
+	struct tplg_tuple_set **s)
+{
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *id, *value;
+	struct tplg_tuple_set *set;
+	unsigned int type, num_tuples = 0;
+	struct tplg_tuple *tuple;
+	long int tuple_val;
+	int len;
+
+	snd_config_get_id(cfg, &id);
+
+	if (strcmp(id, "uuid") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_UUID;
+	else if (strcmp(id, "string") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_STRING;
+	else if (strcmp(id, "bool") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_BOOL;
+	else if (strcmp(id, "byte") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_BYTE;
+	else if (strcmp(id, "short") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_SHORT;
+	else if (strcmp(id, "word") == 0)
+		type = SND_SOC_TPLG_TUPLE_TYPE_WORD;
+	else {
+		SNDERR("error: invalid tuple type '%s'\n", id);
+		return -EINVAL;
+	}
+
+	snd_config_for_each(i, next, cfg)
+		num_tuples++;
+	if (!num_tuples)
+		return 0;
+
+	tplg_dbg("\t %d %s tuples:\n", num_tuples, id);
+	set = calloc(1, sizeof(*set) + num_tuples * sizeof(struct tplg_tuple));
+	if (!set)
+		return -ENOMEM;
+
+	set->type = type;
+
+	snd_config_for_each(i, next, cfg) {
+
+		n = snd_config_iterator_entry(i);
+
+		/* get id */
+		if (snd_config_get_id(n, &id) < 0)
+			continue;
+
+		/* get value */
+		if (snd_config_get_string(n, &value) < 0)
+			continue;
+
+		tuple = &set->tuple[set->num_tuples];
+		elem_copy_text(tuple->token, id,
+				SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+
+		switch (type) {
+		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
+			len = strlen(value);
+			if (len > 16 || len == 0) {
+				SNDERR("error: tuple %s: invalid uuid\n", id);
+				goto err;
+			}
+
+			memcpy(tuple->uuid, value, 16);
+			tplg_dbg("\t\t%s = %s\n", tuple->token, tuple->uuid);
+			break;
+
+		case SND_SOC_TPLG_TUPLE_TYPE_STRING:
+			elem_copy_text(tuple->string, value,
+				SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+			tplg_dbg("\t\t%s = %s\n", tuple->token, tuple->string);
+			break;
+
+		case SND_SOC_TPLG_TUPLE_TYPE_BOOL:
+			if (strcmp(value, "true") == 0)
+				tuple->value = 1;
+			tplg_dbg("\t\t%s = %d\n", tuple->token, tuple->value);
+			break;
+
+		case SND_SOC_TPLG_TUPLE_TYPE_BYTE:
+		case SND_SOC_TPLG_TUPLE_TYPE_SHORT:
+		case SND_SOC_TPLG_TUPLE_TYPE_WORD:
+			tuple_val = strtol(value, NULL, 0);
+			if (tuple_val == LONG_MIN || tuple_val == LONG_MAX
+				|| (type == SND_SOC_TPLG_TUPLE_TYPE_WORD
+					&& tuple_val > 0xffffffff)
+				|| (type == SND_SOC_TPLG_TUPLE_TYPE_SHORT
+					&& tuple_val > 0xffff)
+				|| (type == SND_SOC_TPLG_TUPLE_TYPE_BYTE
+					&& tuple_val > 0xff)) {
+				SNDERR("error: tuple %s: invalid value\n", id);
+				goto err;
+			}
+
+			tuple->value = tuple_val;
+			tplg_dbg("\t\t%s = 0x%x\n", tuple->token, tuple->value);
+			break;
+
+		default:
+			break;
+		}
+
+		set->num_tuples++;
+	}
+
+	*s = set;
+	return 0;
+
+err:
+	free(set);
+	return -EINVAL;
+}
+
+static int parse_tuple_sets(snd_tplg_t *tplg, snd_config_t *cfg,
+	struct tplg_vendor_tuples *tuples)
+{
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *id;
+	unsigned int num_tuple_sets = 0;
+	int err;
+
+	if (snd_config_get_type(cfg) != SND_CONFIG_TYPE_COMPOUND) {
+		SNDERR("error: compound type expected for %s", id);
+		return -EINVAL;
+	}
+
+	snd_config_for_each(i, next, cfg) {
+		num_tuple_sets++;
+	}
+
+	if (!num_tuple_sets)
+		return 0;
+
+	tuples->set = calloc(1, num_tuple_sets * sizeof(void *));
+	if (!tuples->set)
+		return -ENOMEM;
+
+	snd_config_for_each(i, next, cfg) {
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_type(n) != SND_CONFIG_TYPE_COMPOUND) {
+			SNDERR("error: compound type expected for %s, is %d",
+			id, snd_config_get_type(n));
+			return -EINVAL;
+		}
+
+		err = parse_tuple_set(tplg, n, &tuples->set[tuples->num_sets]);
+		if (err < 0)
+			return err;
+
+		/* overlook empty tuple sets */
+		if (tuples->set[tuples->num_sets])
+			tuples->num_sets++;
+	}
+
+	return 0;
+}
+
 /* Parse vendor tokens
  */
 int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
@@ -304,10 +466,71 @@ int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
 	return 0;
 }
 
+/* Parse vendor tuples.
+ */
+int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED)
+{
+	snd_config_iterator_t i, next;
+	snd_config_t *n;
+	const char *id, *value;
+	struct tplg_elem *elem;
+	struct tplg_vendor_tuples *tuples;
+	int err;
+
+	elem = tplg_elem_new_common(tplg, cfg, NULL, SND_TPLG_TYPE_TUPLE);
+	if (!elem)
+		return -ENOMEM;
+
+	tplg_dbg(" Vendor Tuples: %s\n", elem->id);
+
+	tuples = calloc(1, sizeof(*tuples));
+	if (!tuples)
+		return -ENOMEM;
+	elem->tuples = tuples;
+
+	snd_config_for_each(i, next, cfg) {
+
+		n = snd_config_iterator_entry(i);
+		if (snd_config_get_id(n, &id) < 0)
+			continue;
+
+		if (strcmp(id, "tokens") == 0) {
+			if (snd_config_get_string(n, &value) < 0)
+				return -EINVAL;
+			tplg_ref_add(elem, SND_TPLG_TYPE_TOKEN, value);
+			tplg_dbg("\t refer to vendor tokens: %s\n", value);
+		}
+
+		if (strcmp(id, "tuples") == 0) {
+			err = parse_tuple_sets(tplg, n, tuples);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
+
+/* Free handler of tuples */
+void tplg_free_tuples(void *obj)
+{
+	struct tplg_vendor_tuples *tuples = (struct tplg_vendor_tuples *)obj;
+	int i;
+
+	if (!tuples || !tuples->set)
+		return;
+
+	for (i = 0; i < tuples->num_sets; i++)
+		free(tuples->set[i]);
+
+	free(tuples->set);
+}
+
 /* Parse Private data.
  *
  * Object private data can either be from file or defined as bytes, shorts,
- * words.
+ * words, tuples.
  */
 int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 	void *private ATTRIBUTE_UNUSED)
@@ -365,6 +588,14 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 			continue;
 		}
 
+		if (strcmp(id, "tuples") == 0) {
+			if (snd_config_get_string(n, &val) < 0)
+				return -EINVAL;
+			tplg_dbg(" Data: %s\n", val);
+			tplg_ref_add(elem, SND_TPLG_TYPE_TUPLE, val);
+			continue;
+		}
+
 		if (strcmp(id, "index") == 0) {
 			if (snd_config_get_string(n, &val) < 0)
 				return -EINVAL;
diff --git a/src/topology/elem.c b/src/topology/elem.c
index 95e3fd4..50414f0 100644
--- a/src/topology/elem.c
+++ b/src/topology/elem.c
@@ -196,6 +196,10 @@ struct tplg_elem* tplg_elem_new_common(snd_tplg_t *tplg,
 	case SND_TPLG_TYPE_TOKEN:
 		list_add_tail(&elem->list, &tplg->token_list);
 		break;
+	case SND_TPLG_TYPE_TUPLE:
+		list_add_tail(&elem->list, &tplg->tuple_list);
+		elem->free = tplg_free_tuples;
+		break;
 	default:
 		free(elem);
 		return NULL;
diff --git a/src/topology/parser.c b/src/topology/parser.c
index 264abc8..0d967b4 100644
--- a/src/topology/parser.c
+++ b/src/topology/parser.c
@@ -181,6 +181,14 @@ static int tplg_parse_config(snd_tplg_t *tplg, snd_config_t *cfg)
 			continue;
 		}
 
+		if (strcmp(id, "SectionVendorTuples") == 0) {
+			err = tplg_parse_compound(tplg, n, tplg_parse_tuples,
+				NULL);
+			if (err < 0)
+				return err;
+			continue;
+		}
+
 		SNDERR("error: unknown section %s\n", id);
 	}
 	return 0;
@@ -416,6 +424,7 @@ snd_tplg_t *snd_tplg_new(void)
 	INIT_LIST_HEAD(&tplg->enum_list);
 	INIT_LIST_HEAD(&tplg->bytes_ext_list);
 	INIT_LIST_HEAD(&tplg->token_list);
+	INIT_LIST_HEAD(&tplg->tuple_list);
 
 	return tplg;
 }
@@ -436,6 +445,7 @@ void snd_tplg_free(snd_tplg_t *tplg)
 	tplg_elem_free_list(&tplg->enum_list);
 	tplg_elem_free_list(&tplg->bytes_ext_list);
 	tplg_elem_free_list(&tplg->token_list);
+	tplg_elem_free_list(&tplg->tuple_list);
 
 	free(tplg);
 }
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 679bfff..4d59a1f 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -70,6 +70,7 @@ struct snd_tplg {
 	struct list_head text_list;
 	struct list_head pdata_list;
 	struct list_head token_list;
+	struct list_head tuple_list;
 	struct list_head pcm_config_list;
 	struct list_head pcm_caps_list;
 
@@ -97,6 +98,28 @@ struct tplg_vendor_tokens {
 	unsigned int num_tokens;
 	struct tplg_token token[0];
 };
+
+/* element for vendor tuples */
+struct tplg_tuple {
+	char token[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	union {
+		char string[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+		char uuid[16];
+		unsigned int value;
+	};
+};
+
+struct tplg_tuple_set {
+	unsigned int  type; /* uuid, bool, byte, short, word, string*/
+	unsigned int  num_tuples;
+	struct tplg_tuple tuple[0];
+};
+
+struct tplg_vendor_tuples {
+	unsigned int  num_sets;
+	struct tplg_tuple_set **set;
+};
+
 /* topology element */
 struct tplg_elem {
 
@@ -130,6 +153,7 @@ struct tplg_elem {
 		struct snd_soc_tplg_ctl_tlv *tlv;
 		struct snd_soc_tplg_private *data;
 		struct tplg_vendor_tokens *tokens;
+		struct tplg_vendor_tuples *tuples;
 	};
 
 	/* an element may refer to other elements:
@@ -166,6 +190,11 @@ int tplg_parse_data(snd_tplg_t *tplg, snd_config_t *cfg,
 int tplg_parse_tokens(snd_tplg_t *tplg, snd_config_t *cfg,
 	void *private ATTRIBUTE_UNUSED);
 
+int tplg_parse_tuples(snd_tplg_t *tplg, snd_config_t *cfg,
+	void *private ATTRIBUTE_UNUSED);
+
+void tplg_free_tuples(void *obj);
+
 int tplg_parse_control_bytes(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
-- 
2.5.0

[PATCH v2 7/7] topology: Build data objects with tuples

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

For data objects with tuples, the parser will bind the vendor tuples
and tokens, copy the tuples to the private buffer of its parent data
object. Then later the builder will export the vendor tuples as private
binary data for the control or widgets objects.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/topology/data.c b/src/topology/data.c
index 8ce1a0a..7482b82 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -253,6 +253,198 @@ static int tplg_parse_data_hex(snd_config_t *cfg, struct tplg_elem *elem,
 	return ret;
 }
 
+/* get the token integer value from its id */
+static int get_token_value(const char *token_id,
+	struct tplg_vendor_tokens *tokens)
+{
+	int i;
+
+	for (i = 0; i < tokens->num_tokens; i++) {
+		if (strcmp(token_id, tokens->token[i].id) == 0)
+			return tokens->token[i].value;
+	}
+
+	SNDERR("error: cannot find token id '%s'\n", token_id);
+	return -1;
+}
+
+/* get the vendor tokens referred by the vendor tuples */
+static struct tplg_elem *get_tokens(snd_tplg_t *tplg, struct tplg_elem *elem)
+{
+	struct tplg_ref *ref;
+	struct list_head *base, *pos;
+	int err = 0;
+
+	base = &elem->ref_list;
+	list_for_each(pos, base) {
+
+		ref = list_entry(pos, struct tplg_ref, list);
+
+		if (!ref->id || ref->type != SND_TPLG_TYPE_TOKEN)
+			continue;
+
+		if (!ref->elem) {
+			ref->elem = tplg_elem_lookup(&tplg->token_list,
+						ref->id, SND_TPLG_TYPE_TOKEN);
+		}
+
+		return ref->elem;
+	}
+
+	return NULL;
+}
+
+/* check if a data element has tuples */
+static bool has_tuples(struct tplg_elem *elem)
+{
+	struct tplg_ref *ref;
+	struct list_head *base, *pos;
+	int err = 0;
+
+	base = &elem->ref_list;
+	list_for_each(pos, base) {
+
+		ref = list_entry(pos, struct tplg_ref, list);
+		if (ref->id && ref->type == SND_TPLG_TYPE_TUPLE)
+			return true;
+	}
+
+	return false;
+}
+
+/* get size of a tuple element from its type */
+static unsigned int get_tuple_size(int type)
+{
+	switch (type) {
+
+	case SND_SOC_TPLG_TUPLE_TYPE_UUID:
+		return sizeof(struct snd_soc_tplg_vendor_uuid_elem);
+
+	case SND_SOC_TPLG_TUPLE_TYPE_STRING:
+		return sizeof(struct snd_soc_tplg_vendor_string_elem);
+
+	default:
+		return sizeof(struct snd_soc_tplg_vendor_value_elem);
+	}
+}
+
+/* fill a data element's private buffer with its tuples */
+static int copy_tuples(struct tplg_elem *elem,
+	struct tplg_vendor_tuples *tuples, struct tplg_vendor_tokens *tokens)
+{
+	struct snd_soc_tplg_private *priv = elem->data;
+	struct tplg_tuple_set *tuple_set;
+	struct tplg_tuple *tuple;
+	struct snd_soc_tplg_vendor_array *array;
+	struct snd_soc_tplg_vendor_uuid_elem *uuid;
+	struct snd_soc_tplg_vendor_string_elem *string;
+	struct snd_soc_tplg_vendor_value_elem *value;
+	int set_size, size, off;
+	int i, j, token_val;
+
+	if (priv) {
+		SNDERR("error: %s has more data than tuples\n", elem->id);
+		return -EINVAL;
+	}
+
+	size = 0;
+	for (i = 0; i < tuples->num_sets ; i++) {
+		tuple_set = tuples->set[i];
+		set_size = sizeof(struct snd_soc_tplg_vendor_array)
+			+ get_tuple_size(tuple_set->type)
+			* tuple_set->num_tuples;
+		size += set_size;
+		if (size > TPLG_MAX_PRIV_SIZE) {
+			SNDERR("error: data too big %d\n", size);
+			return -EINVAL;
+		}
+
+		if (priv != NULL)
+			priv = realloc(priv, sizeof(*priv) + size);
+		else
+			priv = calloc(1, sizeof(*priv) + size);
+		if (!priv)
+			return -ENOMEM;
+
+		off = priv->size;
+		priv->size = size;
+
+		array = (struct snd_soc_tplg_vendor_array *)(priv->data + off);
+		array->size = set_size;
+		array->type = tuple_set->type;
+		array->num_elems = tuple_set->num_tuples;
+
+		/* fill the private data buffer */
+		for (j = 0; j < tuple_set->num_tuples; j++) {
+			tuple = &tuple_set->tuple[j];
+			token_val = get_token_value(tuple->token, tokens);
+			if (token_val  < 0)
+				return -EINVAL;
+
+			switch (tuple_set->type) {
+			case SND_SOC_TPLG_TUPLE_TYPE_UUID:
+				uuid = &array->uuid[j];
+				uuid->token = token_val;
+				memcpy(uuid->uuid, tuple->uuid, 16);
+				break;
+
+			case SND_SOC_TPLG_TUPLE_TYPE_STRING:
+				string = &array->string[j];
+				string->token = token_val;
+				elem_copy_text(string->string, tuple->string,
+					SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+				break;
+
+			default:
+				value = &array->value[j];
+				value->token = token_val;
+				value->value = tuple->value;
+				break;
+			}
+		}
+	}
+
+	elem->data = priv;
+	return 0;
+}
+
+/* build a data element from its tuples */
+static int build_tuples(snd_tplg_t *tplg, struct tplg_elem *elem)
+{
+	struct tplg_ref *ref;
+	struct list_head *base, *pos;
+	struct tplg_elem *tuples, *tokens;
+
+	base = &elem->ref_list;
+	list_for_each(pos, base) {
+
+		ref = list_entry(pos, struct tplg_ref, list);
+
+		if (!ref->id || ref->type != SND_TPLG_TYPE_TUPLE)
+			continue;
+
+		tplg_dbg("look up tuples %s\n", ref->id);
+
+		if (!ref->elem)
+			ref->elem = tplg_elem_lookup(&tplg->tuple_list,
+						ref->id, SND_TPLG_TYPE_TUPLE);
+		tuples = ref->elem;
+		if (!tuples)
+			return -EINVAL;
+
+		tplg_dbg("found tuples %s\n", tuples->id);
+		tokens = get_tokens(tplg, tuples);
+		if (!tokens)
+			return -EINVAL;
+
+		tplg_dbg("found tokens %s\n", tokens->id);
+		/* a data object can only have one tuples object */
+		return copy_tuples(elem, tuples->tuples, tokens->tokens);
+	}
+
+	return 0;
+}
+
 static int parse_tuple_set(snd_tplg_t *tplg, snd_config_t *cfg,
 	struct tplg_tuple_set **s)
 {
@@ -676,3 +868,24 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 	memcpy(priv->data, ref->data->data, priv_data_size);
 	return 0;
 }
+
+/* check data objects and build those with tuples */
+int tplg_build_data(snd_tplg_t *tplg)
+{
+	struct list_head *base, *pos;
+	struct tplg_elem *elem;
+	int err = 0;
+
+	base = &tplg->pdata_list;
+	list_for_each(pos, base) {
+
+		elem = list_entry(pos, struct tplg_elem, list);
+		if (has_tuples(elem)) {
+			err = build_tuples(tplg, elem);
+			if (err < 0)
+				return err;
+		}
+	}
+
+	return 0;
+}
diff --git a/src/topology/parser.c b/src/topology/parser.c
index 0d967b4..30d91f9 100644
--- a/src/topology/parser.c
+++ b/src/topology/parser.c
@@ -242,6 +242,10 @@ static int tplg_build_integ(snd_tplg_t *tplg)
 {
 	int err;
 
+	err = tplg_build_data(tplg);
+	if (err <  0)
+		return err;
+
 	err = tplg_build_controls(tplg);
 	if (err <  0)
 		return err;
diff --git a/src/topology/tplg_local.h b/src/topology/tplg_local.h
index 4d59a1f..4c601d4 100644
--- a/src/topology/tplg_local.h
+++ b/src/topology/tplg_local.h
@@ -222,6 +222,7 @@ int tplg_parse_be(snd_tplg_t *tplg,
 int tplg_parse_cc(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
+int tplg_build_data(snd_tplg_t *tplg);
 int tplg_build_controls(snd_tplg_t *tplg);
 int tplg_build_widgets(snd_tplg_t *tplg);
 int tplg_build_routes(snd_tplg_t *tplg);
-- 
2.5.0

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Takashi Iwai]

On Wed, 30 Mar 2016 09:11:17 +0200,
mengdong.lin@linux.intel.com wrote:
> 
> +		switch (type) {
> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> +			len = strlen(value);
> +			if (len > 16 || len == 0) {
> +				SNDERR("error: tuple %s: invalid uuid\n", id);
> +				goto err;
> +			}
> +
> +			memcpy(tuple->uuid, value, 16);

This may still overflow :)
How about simply using elem_copy_text()?

> +		case SND_SOC_TPLG_TUPLE_TYPE_BYTE:
> +		case SND_SOC_TPLG_TUPLE_TYPE_SHORT:
> +		case SND_SOC_TPLG_TUPLE_TYPE_WORD:
> +			tuple_val = strtol(value, NULL, 0);
> +			if (tuple_val == LONG_MIN || tuple_val == LONG_MAX
> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_WORD
> +					&& tuple_val > 0xffffffff)

Is the check correct on 32bit architecture?

> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_SHORT
> +					&& tuple_val > 0xffff)
> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_BYTE
> +					&& tuple_val > 0xff)) {

Also, what about negative values?


Takashi

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Mengdong Lin]

On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> On Wed, 30 Mar 2016 09:11:17 +0200,
> mengdong.lin@linux.intel.com wrote:
>>
>> +		switch (type) {
>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
>> +			len = strlen(value);
>> +			if (len > 16 || len == 0) {
>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
>> +				goto err;
>> +			}
>> +
>> +			memcpy(tuple->uuid, value, 16);
>
> This may still overflow :)
> How about simply using elem_copy_text()?

Sorry for the late reply.

Would you mind me using uuid_parse() here?
It can convert an input UUID string into the binary representation.

An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user 
friendly for the text conf file. But this will add dependency on libuuid.

>
>> +		case SND_SOC_TPLG_TUPLE_TYPE_BYTE:
>> +		case SND_SOC_TPLG_TUPLE_TYPE_SHORT:
>> +		case SND_SOC_TPLG_TUPLE_TYPE_WORD:
>> +			tuple_val = strtol(value, NULL, 0);
>> +			if (tuple_val == LONG_MIN || tuple_val == LONG_MAX
>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_WORD
>> +					&& tuple_val > 0xffffffff)
>
> Is the check correct on 32bit architecture?

I'll test on 32 bit machine and will use UINT/USHRT/UCHAR_MAX for range 
here.

>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_SHORT
>> +					&& tuple_val > 0xffff)
>> +				|| (type == SND_SOC_TPLG_TUPLE_TYPE_BYTE
>> +					&& tuple_val > 0xff)) {
>
> Also, what about negative values?

I will add check, we don't expect to have negative value here.

Thanks again for your review.

Regards
Mengdong

>
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Takashi Iwai]

On Tue, 05 Apr 2016 07:47:08 +0200,
Mengdong Lin wrote:
> 
> 
> 
> On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> > On Wed, 30 Mar 2016 09:11:17 +0200,
> > mengdong.lin@linux.intel.com wrote:
> >>
> >> +		switch (type) {
> >> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> >> +			len = strlen(value);
> >> +			if (len > 16 || len == 0) {
> >> +				SNDERR("error: tuple %s: invalid uuid\n", id);
> >> +				goto err;
> >> +			}
> >> +
> >> +			memcpy(tuple->uuid, value, 16);
> >
> > This may still overflow :)
> > How about simply using elem_copy_text()?
> 
> Sorry for the late reply.
> 
> Would you mind me using uuid_parse() here?
> It can convert an input UUID string into the binary representation.
> 
> An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user 
> friendly for the text conf file. But this will add dependency on libuuid.

Additional dependency is no-go, especially when the required change is
so trivial.  It's just a string copy, after all.


thanks,

Takashi

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Mengdong Lin]

On 04/05/2016 02:14 PM, Takashi Iwai wrote:
> On Tue, 05 Apr 2016 07:47:08 +0200,
> Mengdong Lin wrote:
>>
>>
>>
>> On 03/30/2016 03:35 PM, Takashi Iwai wrote:
>>> On Wed, 30 Mar 2016 09:11:17 +0200,
>>> mengdong.lin@linux.intel.com wrote:
>>>>
>>>> +		switch (type) {
>>>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
>>>> +			len = strlen(value);
>>>> +			if (len > 16 || len == 0) {
>>>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
>>>> +				goto err;
>>>> +			}
>>>> +
>>>> +			memcpy(tuple->uuid, value, 16);
>>>
>>> This may still overflow :)
>>> How about simply using elem_copy_text()?
>>
>> Sorry for the late reply.
>>
>> Would you mind me using uuid_parse() here?
>> It can convert an input UUID string into the binary representation.
>>
>> An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user
>> friendly for the text conf file. But this will add dependency on libuuid.
>
> Additional dependency is no-go, especially when the required change is
> so trivial.  It's just a string copy, after all.
>

Maybe we can just use strncpy(dest, src, 16), assuming the strncpy will 
not try to write a "\0" at dest[16] that may cause overflow?

The user need to define uuid without "-" in the UUID string in the text 
conf file.

Now the uuid value in ABI is 16-character array:

/* vendor tuple for uuid */
struct snd_soc_tplg_vendor_uuid_elem {
	__le32 token;
	char uuid[16];
} __attribute__((packed));

The last byte of UUID may not be zero.

Thanks
Mengdong

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Takashi Iwai]

On Tue, 05 Apr 2016 10:53:24 +0200,
Mengdong Lin wrote:
> 
> 
> On 04/05/2016 02:14 PM, Takashi Iwai wrote:
> > On Tue, 05 Apr 2016 07:47:08 +0200,
> > Mengdong Lin wrote:
> >>
> >>
> >>
> >> On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> >>> On Wed, 30 Mar 2016 09:11:17 +0200,
> >>> mengdong.lin@linux.intel.com wrote:
> >>>>
> >>>> +		switch (type) {
> >>>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> >>>> +			len = strlen(value);
> >>>> +			if (len > 16 || len == 0) {
> >>>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
> >>>> +				goto err;
> >>>> +			}
> >>>> +
> >>>> +			memcpy(tuple->uuid, value, 16);
> >>>
> >>> This may still overflow :)
> >>> How about simply using elem_copy_text()?
> >>
> >> Sorry for the late reply.
> >>
> >> Would you mind me using uuid_parse() here?
> >> It can convert an input UUID string into the binary representation.
> >>
> >> An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user
> >> friendly for the text conf file. But this will add dependency on libuuid.
> >
> > Additional dependency is no-go, especially when the required change is
> > so trivial.  It's just a string copy, after all.
> >
> 
> Maybe we can just use strncpy(dest, src, 16), assuming the strncpy will 
> not try to write a "\0" at dest[16] that may cause overflow?

You seem to think of things more complicated than needed.

Just reread your code.  What if a shorter string value is passed there
to memcpy() call?  That's what I suggested as an overflow.


Takashi

Re: [PATCH v2 6/7] topology: Add support for parsing vendor tuples

[Lin, Mengdong]

> -----Original Message-----
> From: alsa-devel-bounces@alsa-project.org
> [mailto:alsa-devel-bounces@alsa-project.org] On Behalf Of Takashi Iwai
> Sent: Tuesday, April 05, 2016 5:38 PM
> To: Mengdong Lin
> Cc: alsa-devel@alsa-project.org; Koul, Vinod; Lin, Mengdong;
> broonie@kernel.org; Ughreja, Rakesh A; Girdwood, Liam R; Shah, Hardik T;
> Prusty, Subhransu S
> Subject: Re: [alsa-devel] [PATCH v2 6/7] topology: Add support for parsing
> vendor tuples
> 
> On Tue, 05 Apr 2016 10:53:24 +0200,
> Mengdong Lin wrote:
> >
> >
> > On 04/05/2016 02:14 PM, Takashi Iwai wrote:
> > > On Tue, 05 Apr 2016 07:47:08 +0200,
> > > Mengdong Lin wrote:
> > >>
> > >>
> > >>
> > >> On 03/30/2016 03:35 PM, Takashi Iwai wrote:
> > >>> On Wed, 30 Mar 2016 09:11:17 +0200,
> mengdong.lin@linux.intel.com
> > >>> wrote:
> > >>>>
> > >>>> +		switch (type) {
> > >>>> +		case SND_SOC_TPLG_TUPLE_TYPE_UUID:
> > >>>> +			len = strlen(value);
> > >>>> +			if (len > 16 || len == 0) {
> > >>>> +				SNDERR("error: tuple %s: invalid uuid\n", id);
> > >>>> +				goto err;
> > >>>> +			}
> > >>>> +
> > >>>> +			memcpy(tuple->uuid, value, 16);
> > >>>
> > >>> This may still overflow :)
> > >>> How about simply using elem_copy_text()?
> > >>
> > >> Sorry for the late reply.
> > >>
> > >> Would you mind me using uuid_parse() here?
> > >> It can convert an input UUID string into the binary representation.
> > >>
> > >> An UUID string link "1b4e28ba-2fa1-11d2-883f-b9a761bde3fb" is user
> > >> friendly for the text conf file. But this will add dependency on libuuid.
> > >
> > > Additional dependency is no-go, especially when the required change
> > > is so trivial.  It's just a string copy, after all.
> > >
> >
> > Maybe we can just use strncpy(dest, src, 16), assuming the strncpy
> > will not try to write a "\0" at dest[16] that may cause overflow?
> 
> You seem to think of things more complicated than needed.
> 
> Just reread your code.  What if a shorter string value is passed there to
> memcpy() call?  That's what I suggested as an overflow.

I misunderstood your point and overlooked this bug. 
I should have checked the string length at first and then use the length in the memcpy.

Thanks again
Mengdong