xml buildups: fix bug introduced by recent changes

rgetz · rgetz · commit a0343f326891 · 2020-04-24T13:09:45.000-04:00
be more pedanic about the differences between strlen (doesn't include
null char) and sizeof (which does include null char).

Now, we calculate the exact length the the string (number of char)
add one for malloc, fill everything out, and then check it make sure
the length is one (the null char) left.

tested on host and local (Zedboard + FMCOMMS2)

Signed-off-by: Robin Getz &lt;robin.getz@analog.com&gt;
diff --git a/channel.c b/channel.c
@@ -179,20 +179,28 @@ void iio_channel_init_finalize(struct iio_channel *chn)
 static char *get_attr_xml(struct iio_channel_attr *attr, size_t *length)
 {
 	char *str;
-	size_t len = strlen(attr->name) + sizeof("<attribute name=\"\" />");
-	if (attr->filename)
-		len += strlen(attr->filename) + sizeof("filename=\"\"");
+	size_t len;
 
+	len = strnlen(attr->name, MAX_ATTR_NAME);
+	len += sizeof("<attribute name=\"\" />") - 1;
+
+	if (attr->filename) {
+		len += strnlen(attr->filename, NAME_MAX);
+		len += sizeof(" filename=\"\"") - 1;
+	}
+
+	*length = len; /* just the chars */
+	len++;         /* room for terminating NULL */
 	str = malloc(len);
 	if (!str)
 		return NULL;
 
-	*length = len - 1; /* Skip the \0 */
 	if (attr->filename)
 		iio_snprintf(str, len, "<attribute name=\"%s\" filename=\"%s\" />",
 				attr->name, attr->filename);
 	else
 		iio_snprintf(str, len, "<attribute name=\"%s\" />", attr->name);
+
 	return str;
 }
 
@@ -231,10 +239,13 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length)
 	size_t *attrs_len, scan_element_len = 0;
 	unsigned int i;
 
-	len = sizeof("<channel id=\"\" type=\"\" ></channel>");
+	len = sizeof("<channel id=\"\" type=\"\" ></channel>") - 1;
 	len += strnlen(chn->id, MAX_CHN_ID);
-	len += chn->is_output ? sizeof("output") : sizeof("input");
-	len += chn->name ? sizeof(" name= ") + strnlen(chn->name, MAX_CHN_NAME) : 0;
+	len += (chn->is_output ? sizeof("output") : sizeof("input")) - 1;
+	if (chn->name) {
+		len += sizeof(" name=\"\"") - 1;
+		len += strnlen(chn->name, MAX_CHN_NAME);
+	}
 
 	if (chn->is_scan_element) {
 		scan_element = get_scan_element(chn, &scan_element_len);
@@ -260,6 +271,7 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length)
 		len += attrs_len[i];
 	}
 
+	len++;  /* room for terminating NULL */
 	str = malloc(len);
 	if (!str)
 		goto err_free_attrs;
@@ -307,7 +319,8 @@ char * iio_channel_get_xml(const struct iio_channel *chn, size_t *length)
 
 	*length = ptr - str;
 
-	if (len < 0) {
+	/* NULL char should be left, and that is it */
+	if (len != 1) {
 		IIO_ERROR("Internal libIIO error: iio_channel_get_xml str length isssue\n");
 		free(str);
 		return NULL;
diff --git a/context.c b/context.c
@@ -89,6 +89,7 @@ char * iio_context_create_xml(const struct iio_context *ctx)
 		}
 	}
 
+	len++; /* room for terminating NULL */
 	str = malloc(len);
 	if (!str) {
 		errno = ENOMEM;
@@ -132,7 +133,7 @@ char * iio_context_create_xml(const struct iio_context *ctx)
 		len -= sizeof("</context>") - 1;
 	}
 
-	if (len < 0) {
+	if (len != 1) {
 		IIO_ERROR("Internal libIIO error: iio_context_create_xml str length isssue\n");
 		free(str);
 		return NULL;
diff --git a/device.c b/device.c
@@ -26,9 +26,12 @@
 
 static char *get_attr_xml(const char *attr, size_t *length, enum iio_attr_type type)
 {
-	size_t len = sizeof("<attribute name=\"\" />") + strlen(attr);
+	size_t len;
 	char *str;
 
+	len = sizeof("<attribute name=\"\" />") - 1;
+	len += strnlen(attr, MAX_ATTR_NAME);
+
 	switch(type){
 		case IIO_ATTR_TYPE_DEVICE:
 			break;
@@ -42,11 +45,12 @@ static char *get_attr_xml(const char *attr, size_t *length, enum iio_attr_type t
 			return NULL;
 	}
 
+	*length = len; /* just the chars */
+	len++; /* room for terminating NULL */
 	str = malloc(len);
 	if (!str)
 		return NULL;
 
-	*length = len - 1; /* Skip the \0 */
 	switch (type) {
 		case IIO_ATTR_TYPE_DEVICE:
 			iio_snprintf(str, len, "<attribute name=\"%s\" />", attr);
@@ -70,7 +74,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length)
 	size_t *attrs_len, *channels_len, *buffer_attrs_len, *debug_attrs_len;
 	unsigned int i, j, k;
 
-	len = sizeof("<device id=\"\" ></device> ") - 1;
+	len = sizeof("<device id=\"\" ></device>") - 1;
 	len += strnlen(dev->id, MAX_DEV_ID);
 	if (dev->name) {
 		len += sizeof(" name=\"\"") - 1;
@@ -146,6 +150,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length)
 		len += debug_attrs_len[k];
 	}
 
+	len++;  /* room for terminating NULL */
 	str = malloc(len);
 	if (!str)
 		goto err_free_debug_attrs;
@@ -222,7 +227,7 @@ char * iio_device_get_xml(const struct iio_device *dev, size_t *length)
 
 	*length = ptr - str;
 
-	if (len < 0) {
+	if (len != 1) {
 		IIO_ERROR("Internal libIIO error: iio_device_get_xml str length isssue\n");
 		free(str);
 		return NULL;
diff --git a/iio-private.h b/iio-private.h
@@ -60,15 +60,30 @@
 #define CLEAR_BIT(addr, bit) \
 	*(((uint32_t *) addr) + BIT_WORD(bit)) &= ~BIT_MASK(bit)
 
-/* 256 is the MAX_NAME (file name) on Linux, 4096 is PAGESIZE */
-#define MAX_CHN_ID 256 /* encoded in the sysfs filename */
-#define MAX_CHN_NAME 256 /* encoded in the sysfs filename */
-#define MAX_DEV_ID 256 /* encoded in the sysfs filename */
-#define MAX_DEV_NAME 256 /* encoded in the sysfs filename */
-#define MAX_CTX_NAME 256    /* nominally "xml" */
-#define MAX_CTX_DESC 256    /* nominally "linux ..." */
-#define MAX_ATTR_NAME 256   /* encoded in the sysfs filename */
-#define MAX_ATTR_VALUE 4096 /* Linux page size, could be anything */
+/* https://pubs.opengroup.org/onlinepubs/009695399/basedefs/limits.h.html
+ * {NAME_MAX} : Maximum number of bytes in a filename
+ * {PATH_MAX} : Maximum number of bytes in a pathname
+ * {PAGESIZE} : Size in bytes of a page
+ * Too bad we work on non-POSIX systems
+ */
+#ifndef NAME_MAX
+#define NAME_MAX 256
+#endif
+#ifndef PATH_MAX
+#define PATH_MAX 4096
+#endif
+#ifndef PAGESIZE
+#define PAGESIZE 4096
+#endif
+
+#define MAX_CHN_ID     NAME_MAX  /* encoded in the sysfs filename */
+#define MAX_CHN_NAME   NAME_MAX  /* encoded in the sysfs filename */
+#define MAX_DEV_ID     NAME_MAX  /* encoded in the sysfs filename */
+#define MAX_DEV_NAME   NAME_MAX  /* encoded in the sysfs filename */
+#define MAX_CTX_NAME   NAME_MAX  /* nominally "xml" */
+#define MAX_CTX_DESC   NAME_MAX  /* nominally "linux ..." */
+#define MAX_ATTR_NAME  NAME_MAX  /* encoded in the sysfs filename */
+#define MAX_ATTR_VALUE PAGESIZE  /* Linux page size, could be anything */
 
 /* ntohl/htonl are a nightmare to use in cross-platform applications,
  * since they are defined in different headers on different platforms.

Original file line number	Diff line number	Diff line change
`@@ -89,6 +89,7 @@ char * iio_context_create_xml(const struct iio_context *ctx)`
`89`	`89`	`}`
`90`	`90`	`}`
`91`	`91`
	`92`	`+ len++; /* room for terminating NULL */`
`92`	`93`	`str = malloc(len);`
`93`	`94`	`if (!str) {`
`94`	`95`	`errno = ENOMEM;`
`@@ -132,7 +133,7 @@ char * iio_context_create_xml(const struct iio_context *ctx)`
`132`	`133`	`len -= sizeof("</context>") - 1;`
`133`	`134`	`}`
`134`	`135`
`135`		`- if (len < 0) {`
	`136`	`+ if (len != 1) {`
`136`	`137`	`IIO_ERROR("Internal libIIO error: iio_context_create_xml str length isssue\n");`
`137`	`138`	`free(str);`
`138`	`139`	`return NULL;`