Skip to contentSkip to conversation

iio-tools out-of-staging cleanup

PATCH Applied Hartmut Knaack 2 participants 62 messages May 31, 2015 – Jun 1, 2015 IIO

v1 of 2 Applied Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover iio-tools out-of-staging cleanup

Now that the iio-tools got moved out of staging, it is about time to improve code quality to common kernel level.

Hartmut Knaack (32):
tools:iio:generic_buffer: fix order of freeing data tools:iio:generic_buffer: free dev_dir_name on exit tools:iio:iio_utils: free scan_el_dir on exit tools:iio: free channel-array completely tools:iio:iio_utils: fix allocation handling tools:iio:generic_buffer: add check before free tools:iio:lsiio: add closedir before exit
tools:iio: save errno first tools:iio:iio_event_monitor: save right errno tools:iio:generic_buffer: fix check of errno tools:iio:generic_buffer: pass up right error code tools:iio:generic_buffer: sign-extend and shift data tools:iio:iio_utils: check amount of matches tools:iio:iio_utils: implement digit calculation tools:iio:iio_utils: mark private function static tools:iio: catch errors in string allocation tools:iio:generic_buffer: catch errors for arguments conversion tools:iio:generic_buffer: add error handling tools:iio:iio_event_monitor: add error handling tools:iio:iio_event_monitor: check if event is fully read tools:iio:iio_utils: add error handling tools:iio:lsiio: add error handling tools:iio:iio_utils: add missing documentation tools:iio: return values directly tools:iio:iio_event_monitor: refactor events output tools:iio:iio_utils: refactor assignment of is_signed tools:iio:iio_utils: move up reset of sysfsfp tools:iio:iio_utils: initialize count during declaration tools:iio: rework program parameters tools:iio:iio_utils: pass strings as const
tools:iio: adjust coding style
tools:iio: rename variables

 tools/iio/generic_buffer.c    | 248 ++++++++++++------
 tools/iio/iio_event_monitor.c |  68 +++--
 tools/iio/iio_utils.c         | 584 ++++++++++++++++++++++++++++++++----------
 tools/iio/iio_utils.h         |  35 +--
 tools/iio/lsiio.c             |  94 ++++---
 5 files changed, 737 insertions(+), 292 deletions(-)
Signature · 1 line
2.3.6
#1 lore raw

PATCH 1/32 tools:iio:generic_buffer: fix order of freeing data

data gets allocated before buffer_access, so it should be freed in reverse order. Otherwise, if allocating buffer_access fails, an attempt to free it would be taken, which should not happen.

---
 tools/iio/generic_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +2 -2
index f805493..15f2a40 100644
@@ -345,10 +345,10 @@ int main(int argc, char **argv)
 
 error_close_buffer_access:
 	close(fp);
-error_free_data:
-	free(data);
 error_free_buffer_access:
 	free(buffer_access);
+error_free_data:
+	free(data);
 error_free_buf_dir_name:
 	free(buf_dir_name);
 error_free_triggername:
Signature · 1 line
2.3.6
maintainer Applied
#2 lore raw

Re: [PATCH 01/32] tools:iio:generic_buffer: fix order of freeing data

Quoted 5 lines · Hartmut Knaack

data gets allocated before buffer_access, so it should be freed in reverse order. Otherwise, if allocating buffer_access fails, an attempt to free it would be taken, which should not happen.

Applied to the togreg branch of iio.git which will get pushed out as testing when I'm on a sensible net connection.

An unplanned work trip for the next few days (hopefully not longer).

Jonathan

Quoted 22 lines
---
 tools/iio/generic_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +2 -2
index f805493..15f2a40 100644
@@ -345,10 +345,10 @@ int main(int argc, char **argv)
 
 error_close_buffer_access:
 	close(fp);
-error_free_data:
-	free(data);
 error_free_buffer_access:
 	free(buffer_access);
+error_free_data:
+	free(data);
 error_free_buf_dir_name:
 	free(buf_dir_name);
 error_free_triggername:
#3 lore raw

PATCH 2/32 tools:iio:generic_buffer: free dev_dir_name on exit

In reply to Hartmut Knaack

Make sure to free dev_dir_name in case of an error or regular exit.

---
 tools/iio/generic_buffer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/iio/generic_buffer.c +3 -1
index 15f2a40..7859ee9 100644
@@ -220,7 +220,7 @@ int main(int argc, char **argv)
 				       "%s-dev%d", device_name, dev_num);
 			if (ret < 0) {
 				ret = -ENOMEM;
-				goto error_ret;
+				goto error_free_dev_dir_name;
 			}
 		}
 
@@ -354,6 +354,8 @@ error_free_buf_dir_name:
 error_free_triggername:
 	if (datardytrigger)
 		free(trigger_name);
+error_free_dev_dir_name:
+	free(dev_dir_name);
 error_ret:
 	return ret;
 }
Signature · 1 line
2.3.6
maintainer Applied
#4 lore raw

Re: [PATCH 02/32] tools:iio:generic_buffer: free dev_dir_name on exit

Quoted 3 lines · Hartmut Knaack

Make sure to free dev_dir_name in case of an error or regular exit.

Applied.

Quoted 27 lines
---
 tools/iio/generic_buffer.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
tools/iio/generic_buffer.c +3 -1
index 15f2a40..7859ee9 100644
@@ -220,7 +220,7 @@ int main(int argc, char **argv)
 				       "%s-dev%d", device_name, dev_num);
 			if (ret < 0) {
 				ret = -ENOMEM;
-				goto error_ret;
+				goto error_free_dev_dir_name;
 			}
 		}
 
@@ -354,6 +354,8 @@ error_free_buf_dir_name:
 error_free_triggername:
 	if (datardytrigger)
 		free(trigger_name);
+error_free_dev_dir_name:
+	free(dev_dir_name);
 error_ret:
 	return ret;
 }
#5 lore raw

PATCH 3/32 tools:iio:iio_utils: free scan_el_dir on exit

In reply to Hartmut Knaack

In the error path, the string scan_el_dir got freed, while it was missing when build_channel_array() finished without errors.

---
 tools/iio/iio_utils.c | 1 +
 1 file changed, 1 insertion(+)
tools/iio/iio_utils.c +1 -0
index 6f64521..f879ad7 100644
@@ -403,6 +403,7 @@ int build_channel_array(const char *device_dir,
 	}
 
 	closedir(dp);
+	free(scan_el_dir);
 	/* reorder so that the array is in index order */
 	bsort_channel_array_by_index(ci_array, *counter);
 
Signature · 1 line
2.3.6
maintainer Applied
#6 lore raw

Re: [PATCH 03/32] tools:iio:iio_utils: free scan_el_dir on exit

Quoted 4 lines · Hartmut Knaack

In the error path, the string scan_el_dir got freed, while it was missing when build_channel_array() finished without errors.

applied.

Quoted 17 lines
---
 tools/iio/iio_utils.c | 1 +
 1 file changed, 1 insertion(+)
tools/iio/iio_utils.c +1 -0
index 6f64521..f879ad7 100644
@@ -403,6 +403,7 @@ int build_channel_array(const char *device_dir,
 	}
 
 	closedir(dp);
+	free(scan_el_dir);
 	/* reorder so that the array is in index order */
 	bsort_channel_array_by_index(ci_array, *counter);
 
#7 lore raw

PATCH 4/32 tools:iio: free channel-array completely

In reply to Hartmut Knaack

In iio_utils.c build_channel_array() dynamically allocates the string generic_name in the current iio_channel_info, which doesn't got freed in case of an error.
This dynamically allocated channel-array is used by generic_buffer, and needs to be freed on the error/exit path.

---
 tools/iio/generic_buffer.c | 8 +++++++-
 tools/iio/iio_utils.c      | 4 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +7 -1
index 7859ee9..0410948 100644
@@ -255,7 +255,7 @@ int main(int argc, char **argv)
 		       "%siio:device%d/buffer", iio_dir, dev_num);
 	if (ret < 0) {
 		ret = -ENOMEM;
-		goto error_free_triggername;
+		goto error_free_channels;
 	}
 
 	if (!notrigger) {
@@ -351,6 +351,12 @@ error_free_data:
 	free(data);
 error_free_buf_dir_name:
 	free(buf_dir_name);
+error_free_channels:
+	for (i = num_channels - 1; i >= 0; i--) {
+		free(channels[i].name);
+		free(channels[i].generic_name);
+	}
+	free(channels);
 error_free_triggername:
 	if (datardytrigger)
 		free(trigger_name);
tools/iio/iio_utils.c +3 -1
index f879ad7..6daf98f 100644
@@ -410,8 +410,10 @@ int build_channel_array(const char *device_dir,
 	return 0;
 
 error_cleanup_array:
-	for (i = count - 1;  i >= 0; i--)
+	for (i = count - 1;  i >= 0; i--) {
 		free((*ci_array)[i].name);
+		free((*ci_array)[i].generic_name);
+	}
 	free(*ci_array);
 error_close_dir:
 	closedir(dp);
Signature · 1 line
2.3.6
maintainer Applied
#8 lore raw

Re: [PATCH 04/32] tools:iio: free channel-array completely

Quoted 7 lines · Hartmut Knaack

In iio_utils.c build_channel_array() dynamically allocates the string generic_name in the current iio_channel_info, which doesn't got freed in case of an error.
This dynamically allocated channel-array is used by generic_buffer, and needs to be freed on the error/exit path.

Applied.

Quoted 48 lines
---
 tools/iio/generic_buffer.c | 8 +++++++-
 tools/iio/iio_utils.c      | 4 +++-
 2 files changed, 10 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +7 -1
index 7859ee9..0410948 100644
@@ -255,7 +255,7 @@ int main(int argc, char **argv)
 		       "%siio:device%d/buffer", iio_dir, dev_num);
 	if (ret < 0) {
 		ret = -ENOMEM;
-		goto error_free_triggername;
+		goto error_free_channels;
 	}
 
 	if (!notrigger) {
@@ -351,6 +351,12 @@ error_free_data:
 	free(data);
 error_free_buf_dir_name:
 	free(buf_dir_name);
+error_free_channels:
+	for (i = num_channels - 1; i >= 0; i--) {
+		free(channels[i].name);
+		free(channels[i].generic_name);
+	}
+	free(channels);
 error_free_triggername:
 	if (datardytrigger)
 		free(trigger_name);
tools/iio/iio_utils.c +3 -1
index f879ad7..6daf98f 100644
@@ -410,8 +410,10 @@ int build_channel_array(const char *device_dir,
 	return 0;
 
 error_cleanup_array:
-	for (i = count - 1;  i >= 0; i--)
+	for (i = count - 1;  i >= 0; i--) {
 		free((*ci_array)[i].name);
+		free((*ci_array)[i].generic_name);
+	}
 	free(*ci_array);
 error_close_dir:
 	closedir(dp);
#9 lore raw

PATCH 5/32 tools:iio:iio_utils: fix allocation handling

In reply to Hartmut Knaack

In build_channel_array(), count needs to be decreased in more places since current->name and current->generic_name would be freed on the error path, although they have not been allocated, yet.
This also requires to free current->name, when it is allocated, but current->generic_name is not yet allocated.

---
 tools/iio/iio_utils.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/iio/iio_utils.c +4 -0
index 6daf98f..1c0ca2f 100644
@@ -334,6 +334,7 @@ int build_channel_array(const char *device_dir,
 			if (sysfsfp == NULL) {
 				free(filename);
 				ret = -errno;
+				count--;
 				goto error_cleanup_array;
 			}
 			fscanf(sysfsfp, "%i", &current_enabled);
@@ -353,6 +354,7 @@ int build_channel_array(const char *device_dir,
 			if (current->name == NULL) {
 				free(filename);
 				ret = -ENOMEM;
+				count--;
 				goto error_cleanup_array;
 			}
 			/* Get the generic and specific name elements */
@@ -360,6 +362,8 @@ int build_channel_array(const char *device_dir,
 						     &current->generic_name);
 			if (ret) {
 				free(filename);
+				free(current->name);
+				count--;
 				goto error_cleanup_array;
 			}
 			ret = asprintf(&filename,
Signature · 1 line
2.3.6
maintainer Applied
#10 lore raw

Re: [PATCH 05/32] tools:iio:iio_utils: fix allocation handling

Quoted 7 lines · Hartmut Knaack

In build_channel_array(), count needs to be decreased in more places since current->name and current->generic_name would be freed on the error path, although they have not been allocated, yet.
This also requires to free current->name, when it is allocated, but current->generic_name is not yet allocated.

Applied.

Quoted 34 lines
---
 tools/iio/iio_utils.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/iio/iio_utils.c +4 -0
index 6daf98f..1c0ca2f 100644
@@ -334,6 +334,7 @@ int build_channel_array(const char *device_dir,
 			if (sysfsfp == NULL) {
 				free(filename);
 				ret = -errno;
+				count--;
 				goto error_cleanup_array;
 			}
 			fscanf(sysfsfp, "%i", &current_enabled);
@@ -353,6 +354,7 @@ int build_channel_array(const char *device_dir,
 			if (current->name == NULL) {
 				free(filename);
 				ret = -ENOMEM;
+				count--;
 				goto error_cleanup_array;
 			}
 			/* Get the generic and specific name elements */
@@ -360,6 +362,8 @@ int build_channel_array(const char *device_dir,
 						     &current->generic_name);
 			if (ret) {
 				free(filename);
+				free(current->name);
+				count--;
 				goto error_cleanup_array;
 			}
 			ret = asprintf(&filename,
#11 lore raw

PATCH 6/32 tools:iio:generic_buffer: add check before free

In reply to Hartmut Knaack

The string trigger_name only gets dynamically allocated, if !notrigger == true. Therefore add this check before freeing the string.

---
 tools/iio/generic_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/generic_buffer.c +1 -1
index 0410948..51ae3df 100644
@@ -358,7 +358,7 @@ error_free_channels:
 	}
 	free(channels);
 error_free_triggername:
-	if (datardytrigger)
+	if (datardytrigger && !notrigger)
 		free(trigger_name);
 error_free_dev_dir_name:
 	free(dev_dir_name);
Signature · 1 line
2.3.6
maintainer
#12 lore raw

Re: [PATCH 06/32] tools:iio:generic_buffer: add check before free

Quoted 4 lines · Hartmut Knaack

The string trigger_name only gets dynamically allocated, if !notrigger == true. Therefore add this check before freeing the string.

Under the notrigger condition, trigger_name is null and free is a noop on a NULL pointer. Could be argued that it is more logical not to free it though!

Quoted 18 lines
---
 tools/iio/generic_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/generic_buffer.c +1 -1
index 0410948..51ae3df 100644
@@ -358,7 +358,7 @@ error_free_channels:
 	}
 	free(channels);
 error_free_triggername:
-	if (datardytrigger)
+	if (datardytrigger && !notrigger)
 		free(trigger_name);
 error_free_dev_dir_name:
 	free(dev_dir_name);
#13 lore raw

PATCH 7/32 tools:iio:lsiio: add closedir before exit

In reply to Hartmut Knaack

In dump_channels() the DIR *dp was left open on exit. Close it and check for errors.

---
 tools/iio/lsiio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/lsiio.c +1 -1
index c585440..65a2385 100644
@@ -56,7 +56,7 @@ static int dump_channels(const char *dev_dir_name)
 			printf("   %-10s\n", ent->d_name);
 		}
 
-	return 0;
+	return (closedir(dp) == -1) ? -errno : 0;
 }
 
 static int dump_one_device(const char *dev_dir_name)
Signature · 1 line
2.3.6
maintainer Applied
#14 lore raw

Re: [PATCH 07/32] tools:iio:lsiio: add closedir before exit

Quoted 4 lines · Hartmut Knaack

In dump_channels() the DIR *dp was left open on exit. Close it and check for errors.

Applied.

Quoted 18 lines
---
 tools/iio/lsiio.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/lsiio.c +1 -1
index c585440..65a2385 100644
@@ -56,7 +56,7 @@ static int dump_channels(const char *dev_dir_name)
 			printf("   %-10s\n", ent->d_name);
 		}
 
-	return 0;
+	return (closedir(dp) == -1) ? -errno : 0;
 }
 
 static int dump_one_device(const char *dev_dir_name)
#15 lore raw

PATCH 8/32 tools:iio: save errno first

In reply to Hartmut Knaack

The man-page of errno states, that errno should be saved before doing any library call, as that call may have changed the value of errno. So, when encountering any error, save errno first.
This patch affects generic_buffer.c, iio_event_monitor.c and iio_utils.c.

---
 tools/iio/generic_buffer.c    |  2 +-
 tools/iio/iio_event_monitor.c |  6 +++---
 tools/iio/iio_utils.c         | 14 +++++++-------
 3 files changed, 11 insertions(+), 11 deletions(-)
tools/iio/generic_buffer.c +1 -1
index 51ae3df..d52a55a 100644
@@ -296,8 +296,8 @@ int main(int argc, char **argv)
 	/* Attempt to open non blocking the access dev */
 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
 	if (fp == -1) { /* If it isn't there make the node */
-		printf("Failed to open %s\n", buffer_access);
 		ret = -errno;
+		printf("Failed to open %s\n", buffer_access);
 		goto error_free_buffer_access;
 	}
 
tools/iio/iio_event_monitor.c +3 -3
index 427c271..f6fdab4 100644
@@ -269,8 +269,8 @@ int main(int argc, char **argv)
 
 	fd = open(chrdev_name, 0);
 	if (fd == -1) {
-		fprintf(stdout, "Failed to open %s\n", chrdev_name);
 		ret = -errno;
+		fprintf(stdout, "Failed to open %s\n", chrdev_name);
 		goto error_free_chrdev_name;
 	}
 
@@ -279,8 +279,8 @@ int main(int argc, char **argv)
 	close(fd);
 
 	if (ret == -1 || event_fd == -1) {
-		fprintf(stdout, "Failed to retrieve event fd\n");
 		ret = -errno;
+		fprintf(stdout, "Failed to retrieve event fd\n");
 		goto error_free_chrdev_name;
 	}
 
@@ -291,8 +291,8 @@ int main(int argc, char **argv)
 				printf("nothing available\n");
 				continue;
 			} else {
-				perror("Failed to read event from device");
 				ret = -errno;
+				perror("Failed to read event from device");
 				break;
 			}
 		}
tools/iio/iio_utils.c +7 -7
index 1c0ca2f..f12bc2e 100644
@@ -129,8 +129,8 @@ int iioutils_get_type(unsigned *is_signed,
 			}
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
-				printf("failed to open %s\n", filename);
 				ret = -errno;
+				printf("failed to open %s\n", filename);
 				goto error_free_filename;
 			}
 
@@ -141,8 +141,8 @@ int iioutils_get_type(unsigned *is_signed,
 				     bits_used,
 				     &padint, shift);
 			if (ret < 0) {
-				printf("failed to pass scan type description\n");
 				ret = -errno;
+				printf("failed to pass scan type description\n");
 				goto error_close_sysfsfp;
 			}
 			*be = (endianchar == 'b');
@@ -332,8 +332,8 @@ int build_channel_array(const char *device_dir,
 			}
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
-				free(filename);
 				ret = -errno;
+				free(filename);
 				count--;
 				goto error_cleanup_array;
 			}
@@ -505,8 +505,8 @@ int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 	sprintf(temp, "%s/%s", basedir, filename);
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
-		printf("failed to open %s\n", temp);
 		ret = -errno;
+		printf("failed to open %s\n", temp);
 		goto error_free;
 	}
 	fprintf(sysfsfp, "%d", val);
@@ -514,8 +514,8 @@ int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
-			printf("failed to open %s\n", temp);
 			ret = -errno;
+			printf("failed to open %s\n", temp);
 			goto error_free;
 		}
 		fscanf(sysfsfp, "%d", &test);
@@ -556,8 +556,8 @@ int _write_sysfs_string(char *filename, char *basedir, char *val, int verify)
 	sprintf(temp, "%s/%s", basedir, filename);
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
-		printf("Could not open %s\n", temp);
 		ret = -errno;
+		printf("Could not open %s\n", temp);
 		goto error_free;
 	}
 	fprintf(sysfsfp, "%s", val);
@@ -565,8 +565,8 @@ int _write_sysfs_string(char *filename, char *basedir, char *val, int verify)
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
-			printf("could not open file to verify\n");
 			ret = -errno;
+			printf("could not open file to verify\n");
 			goto error_free;
 		}
 		fscanf(sysfsfp, "%s", temp);
Signature · 1 line
2.3.6
maintainer Applied
#16 lore raw

Re: [PATCH 08/32] tools:iio: save errno first

Quoted 6 lines · Hartmut Knaack

The man-page of errno states, that errno should be saved before doing any library call, as that call may have changed the value of errno. So, when encountering any error, save errno first.
This patch affects generic_buffer.c, iio_event_monitor.c and iio_utils.c.

Applied,

Not one I ever thought about, but can easily see why the way it was done here is a bad idea. Thanks Hartmut!

Quoted 129 lines
---
 tools/iio/generic_buffer.c    |  2 +-
 tools/iio/iio_event_monitor.c |  6 +++---
 tools/iio/iio_utils.c         | 14 +++++++-------
 3 files changed, 11 insertions(+), 11 deletions(-)
tools/iio/generic_buffer.c +1 -1
index 51ae3df..d52a55a 100644
@@ -296,8 +296,8 @@ int main(int argc, char **argv)
 	/* Attempt to open non blocking the access dev */
 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
 	if (fp == -1) { /* If it isn't there make the node */
-		printf("Failed to open %s\n", buffer_access);
 		ret = -errno;
+		printf("Failed to open %s\n", buffer_access);
 		goto error_free_buffer_access;
 	}
 
tools/iio/iio_event_monitor.c +3 -3
index 427c271..f6fdab4 100644
@@ -269,8 +269,8 @@ int main(int argc, char **argv)
 
 	fd = open(chrdev_name, 0);
 	if (fd == -1) {
-		fprintf(stdout, "Failed to open %s\n", chrdev_name);
 		ret = -errno;
+		fprintf(stdout, "Failed to open %s\n", chrdev_name);
 		goto error_free_chrdev_name;
 	}
 
@@ -279,8 +279,8 @@ int main(int argc, char **argv)
 	close(fd);
 
 	if (ret == -1 || event_fd == -1) {
-		fprintf(stdout, "Failed to retrieve event fd\n");
 		ret = -errno;
+		fprintf(stdout, "Failed to retrieve event fd\n");
 		goto error_free_chrdev_name;
 	}
 
@@ -291,8 +291,8 @@ int main(int argc, char **argv)
 				printf("nothing available\n");
 				continue;
 			} else {
-				perror("Failed to read event from device");
 				ret = -errno;
+				perror("Failed to read event from device");
 				break;
 			}
 		}
tools/iio/iio_utils.c +7 -7
index 1c0ca2f..f12bc2e 100644
@@ -129,8 +129,8 @@ int iioutils_get_type(unsigned *is_signed,
 			}
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
-				printf("failed to open %s\n", filename);
 				ret = -errno;
+				printf("failed to open %s\n", filename);
 				goto error_free_filename;
 			}
 
@@ -141,8 +141,8 @@ int iioutils_get_type(unsigned *is_signed,
 				     bits_used,
 				     &padint, shift);
 			if (ret < 0) {
-				printf("failed to pass scan type description\n");
 				ret = -errno;
+				printf("failed to pass scan type description\n");
 				goto error_close_sysfsfp;
 			}
 			*be = (endianchar == 'b');
@@ -332,8 +332,8 @@ int build_channel_array(const char *device_dir,
 			}
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
-				free(filename);
 				ret = -errno;
+				free(filename);
 				count--;
 				goto error_cleanup_array;
 			}
@@ -505,8 +505,8 @@ int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 	sprintf(temp, "%s/%s", basedir, filename);
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
-		printf("failed to open %s\n", temp);
 		ret = -errno;
+		printf("failed to open %s\n", temp);
 		goto error_free;
 	}
 	fprintf(sysfsfp, "%d", val);
@@ -514,8 +514,8 @@ int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
-			printf("failed to open %s\n", temp);
 			ret = -errno;
+			printf("failed to open %s\n", temp);
 			goto error_free;
 		}
 		fscanf(sysfsfp, "%d", &test);
@@ -556,8 +556,8 @@ int _write_sysfs_string(char *filename, char *basedir, char *val, int verify)
 	sprintf(temp, "%s/%s", basedir, filename);
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
-		printf("Could not open %s\n", temp);
 		ret = -errno;
+		printf("Could not open %s\n", temp);
 		goto error_free;
 	}
 	fprintf(sysfsfp, "%s", val);
@@ -565,8 +565,8 @@ int _write_sysfs_string(char *filename, char *basedir, char *val, int verify)
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
-			printf("could not open file to verify\n");
 			ret = -errno;
+			printf("could not open file to verify\n");
 			goto error_free;
 		}
 		fscanf(sysfsfp, "%s", temp);
#17 lore raw

PATCH 9/32 tools:iio:iio_event_monitor: save right errno

In reply to Hartmut Knaack

Move up error handling code to preserve the errno coming from ioctl(), before it may be changed by close().

---
 tools/iio/iio_event_monitor.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
tools/iio/iio_event_monitor.c +5 -3
index f6fdab4..578390d 100644
@@ -275,15 +275,17 @@ int main(int argc, char **argv)
 	}
 
 	ret = ioctl(fd, IIO_GET_EVENT_FD_IOCTL, &event_fd);
-
-	close(fd);
-
 	if (ret == -1 || event_fd == -1) {
 		ret = -errno;
 		fprintf(stdout, "Failed to retrieve event fd\n");
+		if (close(fd) == -1)
+			perror("Failed to close character device file");
+
 		goto error_free_chrdev_name;
 	}
 
+	close(fd);
+
 	while (true) {
 		ret = read(event_fd, &event, sizeof(event));
 		if (ret == -1) {
Signature · 1 line
2.3.6
maintainer Applied
#18 lore raw

Re: [PATCH 09/32] tools:iio:iio_event_monitor: save right errno

Quoted 4 lines · Hartmut Knaack

Move up error handling code to preserve the errno coming from ioctl(), before it may be changed by close().

Applied.

Quoted 30 lines
---
 tools/iio/iio_event_monitor.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
tools/iio/iio_event_monitor.c +5 -3
index f6fdab4..578390d 100644
@@ -275,15 +275,17 @@ int main(int argc, char **argv)
 	}
 
 	ret = ioctl(fd, IIO_GET_EVENT_FD_IOCTL, &event_fd);
-
-	close(fd);
-
 	if (ret == -1 || event_fd == -1) {
 		ret = -errno;
 		fprintf(stdout, "Failed to retrieve event fd\n");
+		if (close(fd) == -1)
+			perror("Failed to close character device file");
+
 		goto error_free_chrdev_name;
 	}
 
+	close(fd);
+
 	while (true) {
 		ret = read(event_fd, &event, sizeof(event));
 		if (ret == -1) {
#19 lore raw

PATCH 10/32 tools:iio:generic_buffer: fix check of errno

In reply to Hartmut Knaack

Since errno contains the value of any of the defined error names, a negation will not lead to the desired match.

---
 tools/iio/generic_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/generic_buffer.c +1 -1
index d52a55a..7e6f3f1 100644
@@ -321,7 +321,7 @@ int main(int argc, char **argv)
 				 data,
 				 toread*scan_size);
 		if (read_size < 0) {
-			if (errno == -EAGAIN) {
+			if (errno == EAGAIN) {
 				printf("nothing available\n");
 				continue;
 			} else
Signature · 1 line
2.3.6
#20 lore raw

PATCH 11/32 tools:iio:generic_buffer: pass up right error code

find_type_by_name() returns a valid error code in case of an error. Pass this code up instead of an artificial one.

---
 tools/iio/generic_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +2 -2
index 7e6f3f1..a3cf8e3 100644
@@ -202,7 +202,7 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num < 0) {
 		printf("Failed to find the %s\n", device_name);
-		ret = -ENODEV;
+		ret = dev_num;
 		goto error_ret;
 	}
 	printf("iio device number being used is %d\n", dev_num);
@@ -228,7 +228,7 @@ int main(int argc, char **argv)
 		trig_num = find_type_by_name(trigger_name, "trigger");
 		if (trig_num < 0) {
 			printf("Failed to find the trigger %s\n", trigger_name);
-			ret = -ENODEV;
+			ret = trig_num;
 			goto error_free_triggername;
 		}
 		printf("iio trigger number being used is %d\n", trig_num);
Signature · 1 line
2.3.6
maintainer Applied
#21 lore raw

Re: [PATCH 11/32] tools:iio:generic_buffer: pass up right error code

Quoted 4 lines · Hartmut Knaack

find_type_by_name() returns a valid error code in case of an error. Pass this code up instead of an artificial one.

Applied.

Quoted 27 lines
---
 tools/iio/generic_buffer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
tools/iio/generic_buffer.c +2 -2
index 7e6f3f1..a3cf8e3 100644
@@ -202,7 +202,7 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num < 0) {
 		printf("Failed to find the %s\n", device_name);
-		ret = -ENODEV;
+		ret = dev_num;
 		goto error_ret;
 	}
 	printf("iio device number being used is %d\n", dev_num);
@@ -228,7 +228,7 @@ int main(int argc, char **argv)
 		trig_num = find_type_by_name(trigger_name, "trigger");
 		if (trig_num < 0) {
 			printf("Failed to find the trigger %s\n", trigger_name);
-			ret = -ENODEV;
+			ret = trig_num;
 			goto error_free_triggername;
 		}
 		printf("iio trigger number being used is %d\n", trig_num);
#22 lore raw

PATCH 12/32 tools:iio:generic_buffer: sign-extend and shift data

In reply to Hartmut Knaack

Refactor process_scan() to handle signed and unsigned data, respect shifts and the data mask for 2, 4 and 8 byte sized scan elements.

---
 tools/iio/generic_buffer.c | 97 +++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 35 deletions(-)
tools/iio/generic_buffer.c +62 -35
index a3cf8e3..bd974b8 100644
@@ -59,33 +59,80 @@ int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 	return bytes;
 }
 
-void print2byte(int input, struct iio_channel_info *info)
+void print2byte(uint16_t input, struct iio_channel_info *info)
 {
 	/* First swap if incorrect endian */
 	if (info->be)
-		input = be16toh((uint16_t)input);
+		input = be16toh(input);
 	else
-		input = le16toh((uint16_t)input);
+		input = le16toh(input);
 
 	/*
 	 * Shift before conversion to avoid sign extension
 	 * of left aligned data
 	 */
 	input >>= info->shift;
+	input &= info->mask;
 	if (info->is_signed) {
-		int16_t val = input;
+		int16_t val = (int16_t)(input << (16 - info->bits_used)) >>
+			      (16 - info->bits_used);
+		printf("%05f ", ((float)val + info->offset) * info->scale);
+	} else {
+		printf("%05f ", ((float)input + info->offset) * info->scale);
+	}
+}
 
-		val &= (1 << info->bits_used) - 1;
-		val = (int16_t)(val << (16 - info->bits_used)) >>
-			(16 - info->bits_used);
-		printf("%05f ", ((float)val + info->offset)*info->scale);
+void print4byte(uint32_t input, struct iio_channel_info *info)
+{
+	/* First swap if incorrect endian */
+	if (info->be)
+		input = be32toh(input);
+	else
+		input = le32toh(input);
+
+	/*
+	 * Shift before conversion to avoid sign extension
+	 * of left aligned data
+	 */
+	input >>= info->shift;
+	input &= info->mask;
+	if (info->is_signed) {
+		int32_t val = (int32_t)(input << (32 - info->bits_used)) >>
+			      (32 - info->bits_used);
+		printf("%05f ", ((float)val + info->offset) * info->scale);
 	} else {
-		uint16_t val = input;
+		printf("%05f ", ((float)input + info->offset) * info->scale);
+	}
+}
 
-		val &= (1 << info->bits_used) - 1;
-		printf("%05f ", ((float)val + info->offset)*info->scale);
+void print8byte(uint64_t input, struct iio_channel_info *info)
+{
+	/* First swap if incorrect endian */
+	if (info->be)
+		input = be64toh(input);
+	else
+		input = le64toh(input);
+
+	/*
+	 * Shift before conversion to avoid sign extension
+	 * of left aligned data
+	 */
+	input >>= info->shift;
+	input &= info->mask;
+	if (info->is_signed) {
+		int64_t val = (int64_t)(input << (64 - info->bits_used)) >>
+			      (64 - info->bits_used);
+		/* special case for timestamp */
+		if (info->scale == 1.0f && info->offset == 0.0f)
+			printf("%" PRId64 " ", val);
+		else
+			printf("%05f ",
+			       ((float)val + info->offset) * info->scale);
+	} else {
+		printf("%05f ", ((float)input + info->offset) * info->scale);
 	}
 }
+
 /**
  * process_scan() - print out the values in SI units
  * @data:		pointer to the start of the scan
@@ -108,32 +155,12 @@ void process_scan(char *data,
 				   &channels[k]);
 			break;
 		case 4:
-			if (!channels[k].is_signed) {
-				uint32_t val = *(uint32_t *)
-					(data + channels[k].location);
-				printf("%05f ", ((float)val +
-						 channels[k].offset)*
-				       channels[k].scale);
-
-			}
+			print4byte(*(uint32_t *)(data + channels[k].location),
+				   &channels[k]);
 			break;
 		case 8:
-			if (channels[k].is_signed) {
-				int64_t val = *(int64_t *)
-					(data +
-					 channels[k].location);
-				if ((val >> channels[k].bits_used) & 1)
-					val = (val & channels[k].mask) |
-						~channels[k].mask;
-				/* special case for timestamp */
-				if (channels[k].scale == 1.0f &&
-				    channels[k].offset == 0.0f)
-					printf("%" PRId64 " ", val);
-				else
-					printf("%05f ", ((float)val +
-							 channels[k].offset)*
-					       channels[k].scale);
-			}
+			print8byte(*(uint64_t *)(data + channels[k].location),
+				   &channels[k]);
 			break;
 		default:
 			break;
Signature · 1 line
2.3.6
maintainer Applied
#23 lore raw

Re: [PATCH 12/32] tools:iio:generic_buffer: sign-extend and shift data

Quoted 4 lines · Hartmut Knaack

Refactor process_scan() to handle signed and unsigned data, respect shifts and the data mask for 2, 4 and 8 byte sized scan elements.

Nice clean up though diff had fun mixing the lines of code to make the patch appear interesting at times ;)

Applied.

Quoted 138 lines
---
 tools/iio/generic_buffer.c | 97 +++++++++++++++++++++++++++++-----------------
 1 file changed, 62 insertions(+), 35 deletions(-)
tools/iio/generic_buffer.c +62 -35
index a3cf8e3..bd974b8 100644
@@ -59,33 +59,80 @@ int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 	return bytes;
 }
 
-void print2byte(int input, struct iio_channel_info *info)
+void print2byte(uint16_t input, struct iio_channel_info *info)
 {
 	/* First swap if incorrect endian */
 	if (info->be)
-		input = be16toh((uint16_t)input);
+		input = be16toh(input);
 	else
-		input = le16toh((uint16_t)input);
+		input = le16toh(input);
 
 	/*
 	 * Shift before conversion to avoid sign extension
 	 * of left aligned data
 	 */
 	input >>= info->shift;
+	input &= info->mask;
 	if (info->is_signed) {
-		int16_t val = input;
+		int16_t val = (int16_t)(input << (16 - info->bits_used)) >>
+			      (16 - info->bits_used);
+		printf("%05f ", ((float)val + info->offset) * info->scale);
+	} else {
+		printf("%05f ", ((float)input + info->offset) * info->scale);
+	}
+}
 
-		val &= (1 << info->bits_used) - 1;
-		val = (int16_t)(val << (16 - info->bits_used)) >>
-			(16 - info->bits_used);
-		printf("%05f ", ((float)val + info->offset)*info->scale);
+void print4byte(uint32_t input, struct iio_channel_info *info)
+{
+	/* First swap if incorrect endian */
+	if (info->be)
+		input = be32toh(input);
+	else
+		input = le32toh(input);
+
+	/*
+	 * Shift before conversion to avoid sign extension
+	 * of left aligned data
+	 */
+	input >>= info->shift;
+	input &= info->mask;
+	if (info->is_signed) {
+		int32_t val = (int32_t)(input << (32 - info->bits_used)) >>
+			      (32 - info->bits_used);
+		printf("%05f ", ((float)val + info->offset) * info->scale);
 	} else {
-		uint16_t val = input;
+		printf("%05f ", ((float)input + info->offset) * info->scale);
+	}
+}
 
-		val &= (1 << info->bits_used) - 1;
-		printf("%05f ", ((float)val + info->offset)*info->scale);
+void print8byte(uint64_t input, struct iio_channel_info *info)
+{
+	/* First swap if incorrect endian */
+	if (info->be)
+		input = be64toh(input);
+	else
+		input = le64toh(input);
+
+	/*
+	 * Shift before conversion to avoid sign extension
+	 * of left aligned data
+	 */
+	input >>= info->shift;
+	input &= info->mask;
+	if (info->is_signed) {
+		int64_t val = (int64_t)(input << (64 - info->bits_used)) >>
+			      (64 - info->bits_used);
+		/* special case for timestamp */
+		if (info->scale == 1.0f && info->offset == 0.0f)
+			printf("%" PRId64 " ", val);
+		else
+			printf("%05f ",
+			       ((float)val + info->offset) * info->scale);
+	} else {
+		printf("%05f ", ((float)input + info->offset) * info->scale);
 	}
 }
+
 /**
  * process_scan() - print out the values in SI units
  * @data:		pointer to the start of the scan
@@ -108,32 +155,12 @@ void process_scan(char *data,
 				   &channels[k]);
 			break;
 		case 4:
-			if (!channels[k].is_signed) {
-				uint32_t val = *(uint32_t *)
-					(data + channels[k].location);
-				printf("%05f ", ((float)val +
-						 channels[k].offset)*
-				       channels[k].scale);
-
-			}
+			print4byte(*(uint32_t *)(data + channels[k].location),
+				   &channels[k]);
 			break;
 		case 8:
-			if (channels[k].is_signed) {
-				int64_t val = *(int64_t *)
-					(data +
-					 channels[k].location);
-				if ((val >> channels[k].bits_used) & 1)
-					val = (val & channels[k].mask) |
-						~channels[k].mask;
-				/* special case for timestamp */
-				if (channels[k].scale == 1.0f &&
-				    channels[k].offset == 0.0f)
-					printf("%" PRId64 " ", val);
-				else
-					printf("%05f ", ((float)val +
-							 channels[k].offset)*
-					       channels[k].scale);
-			}
+			print8byte(*(uint64_t *)(data + channels[k].location),
+				   &channels[k]);
 			break;
 		default:
 			break;
#24 lore raw

PATCH 13/32 tools:iio:iio_utils: check amount of matches

In reply to Hartmut Knaack

fscanf() usually returns the number of input items successfully matched and assigned, which can be fewer than provided (or even zero). Add a check in iioutils_get_type() to make sure all items are matched.

---
 tools/iio/iio_utils.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/iio/iio_utils.c +4 -0
index f12bc2e..c5b4136 100644
@@ -144,6 +144,10 @@ int iioutils_get_type(unsigned *is_signed,
 				ret = -errno;
 				printf("failed to pass scan type description\n");
 				goto error_close_sysfsfp;
+			} else if (ret != 5) {
+				ret = -EIO;
+				printf("scan type description didn't match\n");
+				goto error_close_sysfsfp;
 			}
 			*be = (endianchar == 'b');
 			*bytes = padint / 8;
Signature · 1 line
2.3.6
maintainer Applied
#25 lore raw

Re: [PATCH 13/32] tools:iio:iio_utils: check amount of matches

Quoted 5 lines · Hartmut Knaack

fscanf() usually returns the number of input items successfully matched and assigned, which can be fewer than provided (or even zero). Add a check in iioutils_get_type() to make sure all items are matched.

Applied.

Quoted 20 lines
---
 tools/iio/iio_utils.c | 4 ++++
 1 file changed, 4 insertions(+)
tools/iio/iio_utils.c +4 -0
index f12bc2e..c5b4136 100644
@@ -144,6 +144,10 @@ int iioutils_get_type(unsigned *is_signed,
 				ret = -errno;
 				printf("failed to pass scan type description\n");
 				goto error_close_sysfsfp;
+			} else if (ret != 5) {
+				ret = -EIO;
+				printf("scan type description didn't match\n");
+				goto error_close_sysfsfp;
 			}
 			*be = (endianchar == 'b');
 			*bytes = padint / 8;
#26 lore raw

PATCH 14/32 tools:iio:iio_utils: implement digit calculation

In reply to Hartmut Knaack

Previously, the return value of sscanf() was treated as an indication of the digits it would have read. Yet, sscanf() only returns the amount of valid matches.
Therefore, introduce a function to calculate the decimal digits of the read number and use this one to commence a colon search, as originally intended.

---
 tools/iio/iio_utils.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
tools/iio/iio_utils.c +31 -4
index c5b4136..60e5ec4 100644
@@ -431,6 +431,18 @@ error_ret:
 	return ret;
 }
 
+int calc_digits(int num)
+{
+	int count = 0;
+
+	while (num != 0) {
+		num /= 10;
+		count++;
+	}
+
+	return count;
+}
+
 /**
  * find_type_by_name() - function to match top level types by name
  * @name: top level type instance name
@@ -441,7 +453,7 @@ error_ret:
 int find_type_by_name(const char *name, const char *type)
 {
 	const struct dirent *ent;
-	int number, numstrlen;
+	int number, numstrlen, ret;
 
 	FILE *nameFile;
 	DIR *dp;
@@ -459,9 +471,19 @@ int find_type_by_name(const char *name, const char *type)
 			strcmp(ent->d_name, "..") != 0 &&
 			strlen(ent->d_name) > strlen(type) &&
 			strncmp(ent->d_name, type, strlen(type)) == 0) {
-			numstrlen = sscanf(ent->d_name + strlen(type),
-					   "%d",
-					   &number);
+			errno = 0;
+			ret = sscanf(ent->d_name + strlen(type), "%d", &number);
+			if (ret < 0) {
+				ret = -errno;
+				printf("failed to read element number\n");
+				goto error_close_dir;
+			} else if (ret != 1) {
+				ret = -EIO;
+				printf("failed to match element number\n");
+				goto error_close_dir;
+			}
+
+			numstrlen = calc_digits(number);
 			/* verify the next character is not a colon */
 			if (strncmp(ent->d_name + strlen(type) + numstrlen,
 					":",
@@ -495,6 +517,11 @@ int find_type_by_name(const char *name, const char *type)
 	}
 	closedir(dp);
 	return -ENODEV;
+
+error_close_dir:
+	if (closedir(dp) == -1)
+		perror("find_type_by_name(): Failed to close directory");
+	return ret;
 }
 
 int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
Signature · 1 line
2.3.6
maintainer Applied
#27 lore raw

Re: [PATCH 14/32] tools:iio:iio_utils: implement digit calculation

Quoted 8 lines · Hartmut Knaack

Previously, the return value of sscanf() was treated as an indication of the digits it would have read. Yet, sscanf() only returns the amount of valid matches.
Therefore, introduce a function to calculate the decimal digits of the read number and use this one to commence a colon search, as originally intended.

Applied. Ouch.

Quoted 72 lines
---
 tools/iio/iio_utils.c | 35 +++++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)
tools/iio/iio_utils.c +31 -4
index c5b4136..60e5ec4 100644
@@ -431,6 +431,18 @@ error_ret:
 	return ret;
 }
 
+int calc_digits(int num)
+{
+	int count = 0;
+
+	while (num != 0) {
+		num /= 10;
+		count++;
+	}
+
+	return count;
+}
+
 /**
  * find_type_by_name() - function to match top level types by name
  * @name: top level type instance name
@@ -441,7 +453,7 @@ error_ret:
 int find_type_by_name(const char *name, const char *type)
 {
 	const struct dirent *ent;
-	int number, numstrlen;
+	int number, numstrlen, ret;
 
 	FILE *nameFile;
 	DIR *dp;
@@ -459,9 +471,19 @@ int find_type_by_name(const char *name, const char *type)
 			strcmp(ent->d_name, "..") != 0 &&
 			strlen(ent->d_name) > strlen(type) &&
 			strncmp(ent->d_name, type, strlen(type)) == 0) {
-			numstrlen = sscanf(ent->d_name + strlen(type),
-					   "%d",
-					   &number);
+			errno = 0;
+			ret = sscanf(ent->d_name + strlen(type), "%d", &number);
+			if (ret < 0) {
+				ret = -errno;
+				printf("failed to read element number\n");
+				goto error_close_dir;
+			} else if (ret != 1) {
+				ret = -EIO;
+				printf("failed to match element number\n");
+				goto error_close_dir;
+			}
+
+			numstrlen = calc_digits(number);
 			/* verify the next character is not a colon */
 			if (strncmp(ent->d_name + strlen(type) + numstrlen,
 					":",
@@ -495,6 +517,11 @@ int find_type_by_name(const char *name, const char *type)
 	}
 	closedir(dp);
 	return -ENODEV;
+
+error_close_dir:
+	if (closedir(dp) == -1)
+		perror("find_type_by_name(): Failed to close directory");
+	return ret;
 }
 
 int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
#28 lore raw

PATCH 15/32 tools:iio:iio_utils: mark private function static

In reply to Hartmut Knaack

Functions _write_sysfs_int() and _write_sysfs_string() are supposed to be called only by public wrappers, so make them static.

---
 tools/iio/iio_utils.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
tools/iio/iio_utils.c +3 -2
index 60e5ec4..812153f 100644
@@ -524,7 +524,7 @@ error_close_dir:
 	return ret;
 }
 
-int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
+static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 {
 	int ret = 0;
 	FILE *sysfsfp;
@@ -574,7 +574,8 @@ int write_sysfs_int_and_verify(char *filename, char *basedir, int val)
 	return _write_sysfs_int(filename, basedir, val, 1);
 }
 
-int _write_sysfs_string(char *filename, char *basedir, char *val, int verify)
+static int _write_sysfs_string(char *filename, char *basedir, char *val,
+			       int verify)
 {
 	int ret = 0;
 	FILE  *sysfsfp;
Signature · 1 line
2.3.6
#29 lore raw

PATCH 16/32 tools:iio: catch errors in string allocation

In reply to Hartmut Knaack

This patch catches errors in string allocation in generic_buffer.c, iio_event_monitor.c, iio_utils.c and lsiio.c.

---
 tools/iio/generic_buffer.c    |  4 +++-
 tools/iio/iio_event_monitor.c |  2 ++
 tools/iio/iio_utils.c         |  9 ++++++---
 tools/iio/lsiio.c             | 15 +++++++++++++--
 4 files changed, 24 insertions(+), 6 deletions(-)
tools/iio/generic_buffer.c +3 -1
index bd974b8..9164b27 100644
@@ -234,7 +234,9 @@ int main(int argc, char **argv)
 	}
 	printf("iio device number being used is %d\n", dev_num);
 
-	asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
+	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
+	if (ret < 0)
+		return -ENOMEM;
 
 	if (!notrigger) {
 		if (trigger_name == NULL) {
tools/iio/iio_event_monitor.c +2 -0
index 578390d..1374374 100644
@@ -265,6 +265,8 @@ int main(int argc, char **argv)
 		/* If we can't find a IIO device by name assume device_name is a
 		   IIO chrdev */
 		chrdev_name = strdup(device_name);
+		if (!chrdev_name)
+			return -ENOMEM;
 	}
 
 	fd = open(chrdev_name, 0);
tools/iio/iio_utils.c +6 -3
index 812153f..f0896f46 100644
@@ -36,7 +36,7 @@ int iioutils_break_up_name(const char *full_name,
 	char *current;
 	char *w, *r;
 	char *working, *prefix = "";
-	int i;
+	int i, ret;
 
 	for (i = 0; i < sizeof(iio_direction) / sizeof(iio_direction[0]); i++)
 		if (!strncmp(full_name, iio_direction[i],
@@ -46,6 +46,9 @@ int iioutils_break_up_name(const char *full_name,
 		}
 
 	current = strdup(full_name + strlen(prefix) + 1);
+	if (!current)
+		return -ENOMEM;
+
 	working = strtok(current, "_\0");
 
 	w = working;
@@ -59,10 +62,10 @@ int iioutils_break_up_name(const char *full_name,
 		r++;
 	}
 	*w = '\0';
-	asprintf(generic_name, "%s_%s", prefix, working);
+	ret = asprintf(generic_name, "%s_%s", prefix, working);
 	free(current);
 
-	return 0;
+	return (ret == -1) ? -ENOMEM : 0;
 }
 
 /**
tools/iio/lsiio.c +13 -2
index 65a2385..daa6c53 100644
@@ -107,7 +107,12 @@ static void dump_devices(void)
 		if (check_prefix(ent->d_name, type_device)) {
 			char *dev_dir_name;
 
-			asprintf(&dev_dir_name, "%s%s", iio_dir, ent->d_name);
+			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
+				     ent->d_name) < 0) {
+				printf("Memory allocation failed\n");
+				goto error_close_dir;
+			}
+
 			dump_one_device(dev_dir_name);
 			free(dev_dir_name);
 			if (verblevel >= VERBLEVEL_SENSORS)
@@ -119,11 +124,17 @@ static void dump_devices(void)
 		if (check_prefix(ent->d_name, type_trigger)) {
 			char *dev_dir_name;
 
-			asprintf(&dev_dir_name, "%s%s", iio_dir, ent->d_name);
+			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
+				     ent->d_name) < 0) {
+				printf("Memory allocation failed\n");
+				goto error_close_dir;
+			}
+
 			dump_one_trigger(dev_dir_name);
 			free(dev_dir_name);
 		}
 	}
+error_close_dir:
 	closedir(dp);
 }
 
Signature · 1 line
2.3.6
maintainer Applied
#30 lore raw

Re: [PATCH 16/32] tools:iio: catch errors in string allocation

Quoted 4 lines · Hartmut Knaack

This patch catches errors in string allocation in generic_buffer.c, iio_event_monitor.c, iio_utils.c and lsiio.c.

Applied.

Thanks

Quoted 109 lines
---
 tools/iio/generic_buffer.c    |  4 +++-
 tools/iio/iio_event_monitor.c |  2 ++
 tools/iio/iio_utils.c         |  9 ++++++---
 tools/iio/lsiio.c             | 15 +++++++++++++--
 4 files changed, 24 insertions(+), 6 deletions(-)
tools/iio/generic_buffer.c +3 -1
index bd974b8..9164b27 100644
@@ -234,7 +234,9 @@ int main(int argc, char **argv)
 	}
 	printf("iio device number being used is %d\n", dev_num);
 
-	asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
+	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
+	if (ret < 0)
+		return -ENOMEM;
 
 	if (!notrigger) {
 		if (trigger_name == NULL) {
tools/iio/iio_event_monitor.c +2 -0
index 578390d..1374374 100644
@@ -265,6 +265,8 @@ int main(int argc, char **argv)
 		/* If we can't find a IIO device by name assume device_name is a
 		   IIO chrdev */
 		chrdev_name = strdup(device_name);
+		if (!chrdev_name)
+			return -ENOMEM;
 	}
 
 	fd = open(chrdev_name, 0);
tools/iio/iio_utils.c +6 -3
index 812153f..f0896f46 100644
@@ -36,7 +36,7 @@ int iioutils_break_up_name(const char *full_name,
 	char *current;
 	char *w, *r;
 	char *working, *prefix = "";
-	int i;
+	int i, ret;
 
 	for (i = 0; i < sizeof(iio_direction) / sizeof(iio_direction[0]); i++)
 		if (!strncmp(full_name, iio_direction[i],
@@ -46,6 +46,9 @@ int iioutils_break_up_name(const char *full_name,
 		}
 
 	current = strdup(full_name + strlen(prefix) + 1);
+	if (!current)
+		return -ENOMEM;
+
 	working = strtok(current, "_\0");
 
 	w = working;
@@ -59,10 +62,10 @@ int iioutils_break_up_name(const char *full_name,
 		r++;
 	}
 	*w = '\0';
-	asprintf(generic_name, "%s_%s", prefix, working);
+	ret = asprintf(generic_name, "%s_%s", prefix, working);
 	free(current);
 
-	return 0;
+	return (ret == -1) ? -ENOMEM : 0;
 }
 
 /**
tools/iio/lsiio.c +13 -2
index 65a2385..daa6c53 100644
@@ -107,7 +107,12 @@ static void dump_devices(void)
 		if (check_prefix(ent->d_name, type_device)) {
 			char *dev_dir_name;
 
-			asprintf(&dev_dir_name, "%s%s", iio_dir, ent->d_name);
+			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
+				     ent->d_name) < 0) {
+				printf("Memory allocation failed\n");
+				goto error_close_dir;
+			}
+
 			dump_one_device(dev_dir_name);
 			free(dev_dir_name);
 			if (verblevel >= VERBLEVEL_SENSORS)
@@ -119,11 +124,17 @@ static void dump_devices(void)
 		if (check_prefix(ent->d_name, type_trigger)) {
 			char *dev_dir_name;
 
-			asprintf(&dev_dir_name, "%s%s", iio_dir, ent->d_name);
+			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
+				     ent->d_name) < 0) {
+				printf("Memory allocation failed\n");
+				goto error_close_dir;
+			}
+
 			dump_one_trigger(dev_dir_name);
 			free(dev_dir_name);
 		}
 	}
+error_close_dir:
 	closedir(dp);
 }
 
#31 lore raw

PATCH 17/32 tools:iio:generic_buffer: catch errors for arguments conversion

In reply to Hartmut Knaack

Add handler to catch errors on conversion of numerical arguments.

---
 tools/iio/generic_buffer.c | 9 +++++++++
 1 file changed, 9 insertions(+)
tools/iio/generic_buffer.c +9 -0
index 9164b27..934b4c9 100644
@@ -206,13 +206,22 @@ int main(int argc, char **argv)
 			noevents = 1;
 			break;
 		case 'c':
+			errno = 0;
 			num_loops = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
 			break;
 		case 'w':
+			errno = 0;
 			timedelay = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
 			break;
 		case 'l':
+			errno = 0;
 			buf_len = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
 			break;
 		case 'g':
 			notrigger = 1;
Signature · 1 line
2.3.6
#32 lore raw

PATCH 18/32 tools:iio:generic_buffer: add error handling

In reply to Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.

---
 tools/iio/generic_buffer.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)
tools/iio/generic_buffer.c +14 -4
index 934b4c9..2f48843 100644
@@ -347,7 +347,14 @@ int main(int argc, char **argv)
 				.events = POLLIN,
 			};
 
-			poll(&pfd, 1, -1);
+			ret = poll(&pfd, 1, -1);
+			if (ret < 0) {
+				ret = -errno;
+				goto error_close_buffer_access;
+			} else if (ret == 0) {
+				continue;
+			}
+
 			toread = buf_len;
 
 		} else {
@@ -378,11 +385,14 @@ int main(int argc, char **argv)
 
 	if (!notrigger)
 		/* Disconnect the trigger - just write a dummy name. */
-		write_sysfs_string("trigger/current_trigger",
-				   dev_dir_name, "NULL");
+		ret = write_sysfs_string("trigger/current_trigger",
+					 dev_dir_name, "NULL");
+		if (ret < 0)
+			printf("Failed to write to %s\n", dev_dir_name);
 
 error_close_buffer_access:
-	close(fp);
+	if (close(fp) == -1)
+		perror("Failed to close buffer");
 error_free_buffer_access:
 	free(buffer_access);
 error_free_data:
Signature · 1 line
2.3.6
#33 lore raw

PATCH 19/32 tools:iio:iio_event_monitor: add error handling

In reply to Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.

---
 tools/iio/iio_event_monitor.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
tools/iio/iio_event_monitor.c +7 -2
index 1374374..1316527 100644
@@ -286,7 +286,10 @@ int main(int argc, char **argv)
 		goto error_free_chrdev_name;
 	}
 
-	close(fd);
+	if (close(fd) == -1)  {
+		ret = -errno;
+		goto error_free_chrdev_name;
+	}
 
 	while (true) {
 		ret = read(event_fd, &event, sizeof(event));
@@ -304,7 +307,9 @@ int main(int argc, char **argv)
 		print_event(&event);
 	}
 
-	close(event_fd);
+	if (close(event_fd) == -1)
+		perror("Failed to close event file");
+
 error_free_chrdev_name:
 	free(chrdev_name);
 error_ret:
Signature · 1 line
2.3.6
maintainer Applied
#34 lore raw

Re: [PATCH 19/32] tools:iio:iio_event_monitor: add error handling

Quoted 4 lines · Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.

Applied.

Quoted 32 lines
---
 tools/iio/iio_event_monitor.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
tools/iio/iio_event_monitor.c +7 -2
index 1374374..1316527 100644
@@ -286,7 +286,10 @@ int main(int argc, char **argv)
 		goto error_free_chrdev_name;
 	}
 
-	close(fd);
+	if (close(fd) == -1)  {
+		ret = -errno;
+		goto error_free_chrdev_name;
+	}
 
 	while (true) {
 		ret = read(event_fd, &event, sizeof(event));
@@ -304,7 +307,9 @@ int main(int argc, char **argv)
 		print_event(&event);
 	}
 
-	close(event_fd);
+	if (close(event_fd) == -1)
+		perror("Failed to close event file");
+
 error_free_chrdev_name:
 	free(chrdev_name);
 error_ret:
maintainer Applied
#35 lore raw

Re: [PATCH 10/32] tools:iio:generic_buffer: fix check of errno

In reply to Hartmut Knaack
Quoted 4 lines · Hartmut Knaack

Since errno contains the value of any of the defined error names, a negation will not lead to the desired match.

Darn these pesky userspace conventions!

Thanks ;)

Applied.

Quoted 18 lines
---
 tools/iio/generic_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/generic_buffer.c +1 -1
index d52a55a..7e6f3f1 100644
@@ -321,7 +321,7 @@ int main(int argc, char **argv)
 				 data,
 				 toread*scan_size);
 		if (read_size < 0) {
-			if (errno == -EAGAIN) {
+			if (errno == EAGAIN) {
 				printf("nothing available\n");
 				continue;
 			} else
#36 lore raw

PATCH 20/32 tools:iio:iio_event_monitor: check if event is fully read

In reply to Hartmut Knaack

Check that the read event is of the expected size.

---
 tools/iio/iio_event_monitor.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/iio/iio_event_monitor.c +5 -0
index 1316527..ecee8ac 100644
@@ -304,6 +304,11 @@ int main(int argc, char **argv)
 			}
 		}
 
+		if (ret != sizeof(event)) {
+			printf("Reading event failed, trying again\n");
+			continue;
+		}
+
 		print_event(&event);
 	}
 
Signature · 1 line
2.3.6
#37 lore raw

PATCH 21/32 tools:iio:iio_utils: add error handling

Add error handling to calls which can indicate a major problem by returning an error code.
This also sets ret to -ENOENT in iioutils_get_type() and iioutils_get_param_float() to indicate if no matching directory entry was found.

---
 tools/iio/iio_utils.c | 265 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 223 insertions(+), 42 deletions(-)
tools/iio/iio_utils.c +223 -42
index f0896f46..e1828d0 100644
@@ -50,6 +50,10 @@ int iioutils_break_up_name(const char *full_name,
 		return -ENOMEM;
 
 	working = strtok(current, "_\0");
+	if (!working) {
+		free(current);
+		return -EINVAL;
+	}
 
 	w = working;
 	r = working;
@@ -117,6 +121,7 @@ int iioutils_get_type(unsigned *is_signed,
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		/*
 		 * Do we allow devices to override a generic name with
@@ -162,7 +167,12 @@ int iioutils_get_type(unsigned *is_signed,
 				*is_signed = 1;
 			else
 				*is_signed = 0;
-			fclose(sysfsfp);
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				printf("Failed to close %s\n", filename);
+				goto error_free_filename;
+			}
+
 			free(filename);
 
 			filename = 0;
@@ -170,12 +180,16 @@ int iioutils_get_type(unsigned *is_signed,
 		}
 error_close_sysfsfp:
 	if (sysfsfp)
-		fclose(sysfsfp);
+		if (fclose(sysfsfp))
+			perror("iioutils_get_type(): Failed to close file");
+
 error_free_filename:
 	if (filename)
 		free(filename);
 error_closedir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("iioutils_get_type(): Failed to close directory");
+
 error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
@@ -215,6 +229,7 @@ int iioutils_get_param_float(float *output,
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		if ((strcmp(builtname, ent->d_name) == 0) ||
 		    (strcmp(builtname_generic, ent->d_name) == 0)) {
@@ -229,14 +244,19 @@ int iioutils_get_param_float(float *output,
 				ret = -errno;
 				goto error_free_filename;
 			}
-			fscanf(sysfsfp, "%f", output);
+			errno = 0;
+			if (fscanf(sysfsfp, "%f", output) != 1)
+				ret = errno ? -errno : -ENODATA;
+
 			break;
 		}
 error_free_filename:
 	if (filename)
 		free(filename);
 error_closedir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("iioutils_get_param_float(): Failed to close directory");
+
 error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
@@ -310,10 +330,24 @@ int build_channel_array(const char *device_dir,
 				free(filename);
 				goto error_close_dir;
 			}
-			fscanf(sysfsfp, "%i", &ret);
+			errno = 0;
+			if (fscanf(sysfsfp, "%i", &ret) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				if (fclose(sysfsfp))
+					perror("build_channel_array(): Failed to close file");
+
+				free(filename);
+				goto error_close_dir;
+			}
+
 			if (ret == 1)
 				(*counter)++;
-			fclose(sysfsfp);
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				goto error_close_dir;
+			}
+
 			free(filename);
 		}
 	*ci_array = malloc(sizeof(**ci_array) * (*counter));
@@ -344,8 +378,20 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
-			fscanf(sysfsfp, "%i", &current_enabled);
-			fclose(sysfsfp);
+			errno = 0;
+			if (fscanf(sysfsfp, "%i", &current_enabled) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				free(filename);
+				count--;
+				goto error_cleanup_array;
+			}
+
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				count--;
+				goto error_cleanup_array;
+			}
 
 			if (!current_enabled) {
 				free(filename);
@@ -383,8 +429,29 @@ int build_channel_array(const char *device_dir,
 				goto error_cleanup_array;
 			}
 			sysfsfp = fopen(filename, "r");
-			fscanf(sysfsfp, "%u", &current->index);
-			fclose(sysfsfp);
+			if (sysfsfp == NULL) {
+				ret = -errno;
+				printf("failed to open %s\n", filename);
+				free(filename);
+				goto error_cleanup_array;
+			}
+
+			errno = 0;
+			if (fscanf(sysfsfp, "%u", &current->index) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				if (fclose(sysfsfp))
+					perror("build_channel_array(): Failed to close file");
+
+				free(filename);
+				goto error_cleanup_array;
+			}
+
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				goto error_cleanup_array;
+			}
+
 			free(filename);
 			/* Find the scale */
 			ret = iioutils_get_param_float(&current->scale,
@@ -410,10 +477,16 @@ int build_channel_array(const char *device_dir,
 						device_dir,
 						current->name,
 						current->generic_name);
+			if (ret < 0)
+				goto error_cleanup_array;
 		}
 	}
 
-	closedir(dp);
+	if (closedir(dp) == -1) {
+		ret = -errno;
+		goto error_cleanup_array;
+	}
+
 	free(scan_el_dir);
 	/* reorder so that the array is in index order */
 	bsort_channel_array_by_index(ci_array, *counter);
@@ -427,7 +500,10 @@ error_cleanup_array:
 	}
 	free(*ci_array);
 error_close_dir:
-	closedir(dp);
+	if (dp)
+		if (closedir(dp) == -1)
+			perror("build_channel_array(): Failed to close dir");
+
 error_free_name:
 	free(scan_el_dir);
 error_ret:
@@ -496,29 +572,45 @@ int find_type_by_name(const char *name, const char *type)
 						+ numstrlen
 						+ 6);
 				if (filename == NULL) {
-					closedir(dp);
-					return -ENOMEM;
+					ret = -ENOMEM;
+					goto error_close_dir;
+				}
+
+				ret = sprintf(filename, "%s%s%d/name", iio_dir,
+					      type, number);
+				if (ret < 0) {
+					free(filename);
+					goto error_close_dir;
 				}
-				sprintf(filename, "%s%s%d/name",
-					iio_dir,
-					type,
-					number);
+
 				nameFile = fopen(filename, "r");
 				if (!nameFile) {
 					free(filename);
 					continue;
 				}
 				free(filename);
-				fscanf(nameFile, "%s", thisname);
-				fclose(nameFile);
+				errno = 0;
+				if (fscanf(nameFile, "%s", thisname) != 1) {
+					ret = errno ? -errno : -ENODATA;
+					goto error_close_dir;
+				}
+
+				if (fclose(nameFile)) {
+					ret = -errno;
+					goto error_close_dir;
+				}
+
 				if (strcmp(name, thisname) == 0) {
-					closedir(dp);
+					if (closedir(dp) == -1)
+						return -errno;
 					return number;
 				}
 			}
 		}
 	}
-	closedir(dp);
+	if (closedir(dp) == -1)
+		return -errno;
+
 	return -ENODEV;
 
 error_close_dir:
@@ -536,15 +628,29 @@ static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 
 	if (temp == NULL)
 		return -ENOMEM;
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		printf("failed to open %s\n", temp);
 		goto error_free;
 	}
-	fprintf(sysfsfp, "%d", val);
-	fclose(sysfsfp);
+	ret = fprintf(sysfsfp, "%d", val);
+	if (ret < 0) {
+		if (fclose(sysfsfp))
+			perror("_write_sysfs_int(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp)) {
+		ret = -errno;
+		goto error_free;
+	}
+
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
@@ -552,8 +658,19 @@ static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 			printf("failed to open %s\n", temp);
 			goto error_free;
 		}
-		fscanf(sysfsfp, "%d", &test);
-		fclose(sysfsfp);
+		if (fscanf(sysfsfp, "%d", &test) != 1) {
+			ret = errno ? -errno : -ENODATA;
+			if (fclose(sysfsfp))
+				perror("_write_sysfs_int(): Failed to close dir");
+
+			goto error_free;
+		}
+
+		if (fclose(sysfsfp)) {
+			ret = -errno;
+			goto error_free;
+		}
+
 		if (test != val) {
 			printf("Possible failure in int write %d to %s%s\n",
 				val,
@@ -588,15 +705,29 @@ static int _write_sysfs_string(char *filename, char *basedir, char *val,
 		printf("Memory allocation failed\n");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		printf("Could not open %s\n", temp);
 		goto error_free;
 	}
-	fprintf(sysfsfp, "%s", val);
-	fclose(sysfsfp);
+	ret = fprintf(sysfsfp, "%s", val);
+	if (ret < 0) {
+		if (fclose(sysfsfp))
+			perror("_write_sysfs_string(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp)) {
+		ret = -errno;
+		goto error_free;
+	}
+
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
@@ -604,8 +735,19 @@ static int _write_sysfs_string(char *filename, char *basedir, char *val,
 			printf("could not open file to verify\n");
 			goto error_free;
 		}
-		fscanf(sysfsfp, "%s", temp);
-		fclose(sysfsfp);
+		if (fscanf(sysfsfp, "%s", temp) != 1) {
+			ret = errno ? -errno : -ENODATA;
+			if (fclose(sysfsfp))
+				perror("_write_sysfs_string(): Failed to close dir");
+
+			goto error_free;
+		}
+
+		if (fclose(sysfsfp)) {
+			ret = -errno;
+			goto error_free;
+		}
+
 		if (strcmp(temp, val) != 0) {
 			printf("Possible failure in string write of %s "
 				"Should be %s "
@@ -649,14 +791,27 @@ int read_sysfs_posint(char *filename, char *basedir)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%d\n", &ret);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%d\n", &ret) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_posint(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
@@ -672,14 +827,27 @@ int read_sysfs_float(char *filename, char *basedir, float *val)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%f\n", val);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%f\n", val) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_float(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
@@ -695,14 +863,27 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%s\n", str);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%s\n", str) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_string(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
Signature · 1 line
2.3.6
maintainer Applied
#38 lore raw

Re: [PATCH 21/32] tools:iio:iio_utils: add error handling

Quoted 7 lines · Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.
This also sets ret to -ENOENT in iioutils_get_type() and iioutils_get_param_float() to indicate if no matching directory entry was found.

Applied.

Quoted 462 lines
---
 tools/iio/iio_utils.c | 265 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 223 insertions(+), 42 deletions(-)
tools/iio/iio_utils.c +223 -42
index f0896f46..e1828d0 100644
@@ -50,6 +50,10 @@ int iioutils_break_up_name(const char *full_name,
 		return -ENOMEM;
 
 	working = strtok(current, "_\0");
+	if (!working) {
+		free(current);
+		return -EINVAL;
+	}
 
 	w = working;
 	r = working;
@@ -117,6 +121,7 @@ int iioutils_get_type(unsigned *is_signed,
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		/*
 		 * Do we allow devices to override a generic name with
@@ -162,7 +167,12 @@ int iioutils_get_type(unsigned *is_signed,
 				*is_signed = 1;
 			else
 				*is_signed = 0;
-			fclose(sysfsfp);
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				printf("Failed to close %s\n", filename);
+				goto error_free_filename;
+			}
+
 			free(filename);
 
 			filename = 0;
@@ -170,12 +180,16 @@ int iioutils_get_type(unsigned *is_signed,
 		}
 error_close_sysfsfp:
 	if (sysfsfp)
-		fclose(sysfsfp);
+		if (fclose(sysfsfp))
+			perror("iioutils_get_type(): Failed to close file");
+
 error_free_filename:
 	if (filename)
 		free(filename);
 error_closedir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("iioutils_get_type(): Failed to close directory");
+
 error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
@@ -215,6 +229,7 @@ int iioutils_get_param_float(float *output,
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		if ((strcmp(builtname, ent->d_name) == 0) ||
 		    (strcmp(builtname_generic, ent->d_name) == 0)) {
@@ -229,14 +244,19 @@ int iioutils_get_param_float(float *output,
 				ret = -errno;
 				goto error_free_filename;
 			}
-			fscanf(sysfsfp, "%f", output);
+			errno = 0;
+			if (fscanf(sysfsfp, "%f", output) != 1)
+				ret = errno ? -errno : -ENODATA;
+
 			break;
 		}
 error_free_filename:
 	if (filename)
 		free(filename);
 error_closedir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("iioutils_get_param_float(): Failed to close directory");
+
 error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
@@ -310,10 +330,24 @@ int build_channel_array(const char *device_dir,
 				free(filename);
 				goto error_close_dir;
 			}
-			fscanf(sysfsfp, "%i", &ret);
+			errno = 0;
+			if (fscanf(sysfsfp, "%i", &ret) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				if (fclose(sysfsfp))
+					perror("build_channel_array(): Failed to close file");
+
+				free(filename);
+				goto error_close_dir;
+			}
+
 			if (ret == 1)
 				(*counter)++;
-			fclose(sysfsfp);
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				goto error_close_dir;
+			}
+
 			free(filename);
 		}
 	*ci_array = malloc(sizeof(**ci_array) * (*counter));
@@ -344,8 +378,20 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
-			fscanf(sysfsfp, "%i", &current_enabled);
-			fclose(sysfsfp);
+			errno = 0;
+			if (fscanf(sysfsfp, "%i", &current_enabled) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				free(filename);
+				count--;
+				goto error_cleanup_array;
+			}
+
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				count--;
+				goto error_cleanup_array;
+			}
 
 			if (!current_enabled) {
 				free(filename);
@@ -383,8 +429,29 @@ int build_channel_array(const char *device_dir,
 				goto error_cleanup_array;
 			}
 			sysfsfp = fopen(filename, "r");
-			fscanf(sysfsfp, "%u", &current->index);
-			fclose(sysfsfp);
+			if (sysfsfp == NULL) {
+				ret = -errno;
+				printf("failed to open %s\n", filename);
+				free(filename);
+				goto error_cleanup_array;
+			}
+
+			errno = 0;
+			if (fscanf(sysfsfp, "%u", &current->index) != 1) {
+				ret = errno ? -errno : -ENODATA;
+				if (fclose(sysfsfp))
+					perror("build_channel_array(): Failed to close file");
+
+				free(filename);
+				goto error_cleanup_array;
+			}
+
+			if (fclose(sysfsfp)) {
+				ret = -errno;
+				free(filename);
+				goto error_cleanup_array;
+			}
+
 			free(filename);
 			/* Find the scale */
 			ret = iioutils_get_param_float(&current->scale,
@@ -410,10 +477,16 @@ int build_channel_array(const char *device_dir,
 						device_dir,
 						current->name,
 						current->generic_name);
+			if (ret < 0)
+				goto error_cleanup_array;
 		}
 	}
 
-	closedir(dp);
+	if (closedir(dp) == -1) {
+		ret = -errno;
+		goto error_cleanup_array;
+	}
+
 	free(scan_el_dir);
 	/* reorder so that the array is in index order */
 	bsort_channel_array_by_index(ci_array, *counter);
@@ -427,7 +500,10 @@ error_cleanup_array:
 	}
 	free(*ci_array);
 error_close_dir:
-	closedir(dp);
+	if (dp)
+		if (closedir(dp) == -1)
+			perror("build_channel_array(): Failed to close dir");
+
 error_free_name:
 	free(scan_el_dir);
 error_ret:
@@ -496,29 +572,45 @@ int find_type_by_name(const char *name, const char *type)
 						+ numstrlen
 						+ 6);
 				if (filename == NULL) {
-					closedir(dp);
-					return -ENOMEM;
+					ret = -ENOMEM;
+					goto error_close_dir;
+				}
+
+				ret = sprintf(filename, "%s%s%d/name", iio_dir,
+					      type, number);
+				if (ret < 0) {
+					free(filename);
+					goto error_close_dir;
 				}
-				sprintf(filename, "%s%s%d/name",
-					iio_dir,
-					type,
-					number);
+
 				nameFile = fopen(filename, "r");
 				if (!nameFile) {
 					free(filename);
 					continue;
 				}
 				free(filename);
-				fscanf(nameFile, "%s", thisname);
-				fclose(nameFile);
+				errno = 0;
+				if (fscanf(nameFile, "%s", thisname) != 1) {
+					ret = errno ? -errno : -ENODATA;
+					goto error_close_dir;
+				}
+
+				if (fclose(nameFile)) {
+					ret = -errno;
+					goto error_close_dir;
+				}
+
 				if (strcmp(name, thisname) == 0) {
-					closedir(dp);
+					if (closedir(dp) == -1)
+						return -errno;
 					return number;
 				}
 			}
 		}
 	}
-	closedir(dp);
+	if (closedir(dp) == -1)
+		return -errno;
+
 	return -ENODEV;
 
 error_close_dir:
@@ -536,15 +628,29 @@ static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 
 	if (temp == NULL)
 		return -ENOMEM;
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		printf("failed to open %s\n", temp);
 		goto error_free;
 	}
-	fprintf(sysfsfp, "%d", val);
-	fclose(sysfsfp);
+	ret = fprintf(sysfsfp, "%d", val);
+	if (ret < 0) {
+		if (fclose(sysfsfp))
+			perror("_write_sysfs_int(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp)) {
+		ret = -errno;
+		goto error_free;
+	}
+
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
@@ -552,8 +658,19 @@ static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
 			printf("failed to open %s\n", temp);
 			goto error_free;
 		}
-		fscanf(sysfsfp, "%d", &test);
-		fclose(sysfsfp);
+		if (fscanf(sysfsfp, "%d", &test) != 1) {
+			ret = errno ? -errno : -ENODATA;
+			if (fclose(sysfsfp))
+				perror("_write_sysfs_int(): Failed to close dir");
+
+			goto error_free;
+		}
+
+		if (fclose(sysfsfp)) {
+			ret = -errno;
+			goto error_free;
+		}
+
 		if (test != val) {
 			printf("Possible failure in int write %d to %s%s\n",
 				val,
@@ -588,15 +705,29 @@ static int _write_sysfs_string(char *filename, char *basedir, char *val,
 		printf("Memory allocation failed\n");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "w");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		printf("Could not open %s\n", temp);
 		goto error_free;
 	}
-	fprintf(sysfsfp, "%s", val);
-	fclose(sysfsfp);
+	ret = fprintf(sysfsfp, "%s", val);
+	if (ret < 0) {
+		if (fclose(sysfsfp))
+			perror("_write_sysfs_string(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp)) {
+		ret = -errno;
+		goto error_free;
+	}
+
 	if (verify) {
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
@@ -604,8 +735,19 @@ static int _write_sysfs_string(char *filename, char *basedir, char *val,
 			printf("could not open file to verify\n");
 			goto error_free;
 		}
-		fscanf(sysfsfp, "%s", temp);
-		fclose(sysfsfp);
+		if (fscanf(sysfsfp, "%s", temp) != 1) {
+			ret = errno ? -errno : -ENODATA;
+			if (fclose(sysfsfp))
+				perror("_write_sysfs_string(): Failed to close dir");
+
+			goto error_free;
+		}
+
+		if (fclose(sysfsfp)) {
+			ret = -errno;
+			goto error_free;
+		}
+
 		if (strcmp(temp, val) != 0) {
 			printf("Possible failure in string write of %s "
 				"Should be %s "
@@ -649,14 +791,27 @@ int read_sysfs_posint(char *filename, char *basedir)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%d\n", &ret);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%d\n", &ret) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_posint(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
@@ -672,14 +827,27 @@ int read_sysfs_float(char *filename, char *basedir, float *val)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%f\n", val);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%f\n", val) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_float(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
@@ -695,14 +863,27 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
-	sprintf(temp, "%s/%s", basedir, filename);
+	ret = sprintf(temp, "%s/%s", basedir, filename);
+	if (ret < 0)
+		goto error_free;
+
 	sysfsfp = fopen(temp, "r");
 	if (sysfsfp == NULL) {
 		ret = -errno;
 		goto error_free;
 	}
-	fscanf(sysfsfp, "%s\n", str);
-	fclose(sysfsfp);
+	errno = 0;
+	if (fscanf(sysfsfp, "%s\n", str) != 1) {
+		ret = errno ? -errno : -ENODATA;
+		if (fclose(sysfsfp))
+			perror("read_sysfs_string(): Failed to close dir");
+
+		goto error_free;
+	}
+
+	if (fclose(sysfsfp))
+		ret = -errno;
+
 error_free:
 	free(temp);
 	return ret;
#39 lore raw

PATCH 22/32 tools:iio:lsiio: add error handling

In reply to Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.
This also involves to change the type of dump_devices() from void to int.

---
 tools/iio/lsiio.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)
tools/iio/lsiio.c +32 -12
index daa6c53..b1089ad 100644
@@ -69,7 +69,10 @@ static int dump_one_device(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
-	read_sysfs_string("name", dev_dir_name, name);
+	retval = read_sysfs_string("name", dev_dir_name, name);
+	if (retval)
+		return retval;
+
 	printf("Device %03d: %s\n", dev_idx, name);
 
 	if (verblevel >= VERBLEVEL_SENSORS)
@@ -87,20 +90,24 @@ static int dump_one_trigger(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
-	read_sysfs_string("name", dev_dir_name, name);
+	retval = read_sysfs_string("name", dev_dir_name, name);
+	if (retval)
+		return retval;
+
 	printf("Trigger %03d: %s\n", dev_idx, name);
 	return 0;
 }
 
-static void dump_devices(void)
+static int dump_devices(void)
 {
 	const struct dirent *ent;
+	int ret;
 	DIR *dp;
 
 	dp = opendir(iio_dir);
 	if (dp == NULL) {
 		printf("No industrial I/O devices available\n");
-		return;
+		return -ENODEV;
 	}
 
 	while (ent = readdir(dp), ent != NULL) {
@@ -109,11 +116,16 @@ static void dump_devices(void)
 
 			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
 				     ent->d_name) < 0) {
-				printf("Memory allocation failed\n");
+				ret = -ENOMEM;
+				goto error_close_dir;
+			}
+
+			ret = dump_one_device(dev_dir_name);
+			if (ret) {
+				free(dev_dir_name);
 				goto error_close_dir;
 			}
 
-			dump_one_device(dev_dir_name);
 			free(dev_dir_name);
 			if (verblevel >= VERBLEVEL_SENSORS)
 				printf("\n");
@@ -126,16 +138,26 @@ static void dump_devices(void)
 
 			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
 				     ent->d_name) < 0) {
-				printf("Memory allocation failed\n");
+				ret = -ENOMEM;
+				goto error_close_dir;
+			}
+
+			ret = dump_one_trigger(dev_dir_name);
+			if (ret) {
+				free(dev_dir_name);
 				goto error_close_dir;
 			}
 
-			dump_one_trigger(dev_dir_name);
 			free(dev_dir_name);
 		}
 	}
+	return (closedir(dp) == -1) ? -errno : 0;
+
 error_close_dir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("dump_devices(): Failed to close directory");
+
+	return ret;
 }
 
 int main(int argc, char **argv)
@@ -163,7 +185,5 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	dump_devices();
-
-	return 0;
+	return dump_devices();
 }
Signature · 1 line
2.3.6
maintainer Applied
#40 lore raw

Re: [PATCH 22/32] tools:iio:lsiio: add error handling

Quoted 5 lines · Hartmut Knaack

Add error handling to calls which can indicate a major problem by returning an error code.
This also involves to change the type of dump_devices() from void to int.

Applied.

Quoted 107 lines
---
 tools/iio/lsiio.c | 44 ++++++++++++++++++++++++++++++++------------
 1 file changed, 32 insertions(+), 12 deletions(-)
tools/iio/lsiio.c +32 -12
index daa6c53..b1089ad 100644
@@ -69,7 +69,10 @@ static int dump_one_device(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
-	read_sysfs_string("name", dev_dir_name, name);
+	retval = read_sysfs_string("name", dev_dir_name, name);
+	if (retval)
+		return retval;
+
 	printf("Device %03d: %s\n", dev_idx, name);
 
 	if (verblevel >= VERBLEVEL_SENSORS)
@@ -87,20 +90,24 @@ static int dump_one_trigger(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
-	read_sysfs_string("name", dev_dir_name, name);
+	retval = read_sysfs_string("name", dev_dir_name, name);
+	if (retval)
+		return retval;
+
 	printf("Trigger %03d: %s\n", dev_idx, name);
 	return 0;
 }
 
-static void dump_devices(void)
+static int dump_devices(void)
 {
 	const struct dirent *ent;
+	int ret;
 	DIR *dp;
 
 	dp = opendir(iio_dir);
 	if (dp == NULL) {
 		printf("No industrial I/O devices available\n");
-		return;
+		return -ENODEV;
 	}
 
 	while (ent = readdir(dp), ent != NULL) {
@@ -109,11 +116,16 @@ static void dump_devices(void)
 
 			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
 				     ent->d_name) < 0) {
-				printf("Memory allocation failed\n");
+				ret = -ENOMEM;
+				goto error_close_dir;
+			}
+
+			ret = dump_one_device(dev_dir_name);
+			if (ret) {
+				free(dev_dir_name);
 				goto error_close_dir;
 			}
 
-			dump_one_device(dev_dir_name);
 			free(dev_dir_name);
 			if (verblevel >= VERBLEVEL_SENSORS)
 				printf("\n");
@@ -126,16 +138,26 @@ static void dump_devices(void)
 
 			if (asprintf(&dev_dir_name, "%s%s", iio_dir,
 				     ent->d_name) < 0) {
-				printf("Memory allocation failed\n");
+				ret = -ENOMEM;
+				goto error_close_dir;
+			}
+
+			ret = dump_one_trigger(dev_dir_name);
+			if (ret) {
+				free(dev_dir_name);
 				goto error_close_dir;
 			}
 
-			dump_one_trigger(dev_dir_name);
 			free(dev_dir_name);
 		}
 	}
+	return (closedir(dp) == -1) ? -errno : 0;
+
 error_close_dir:
-	closedir(dp);
+	if (closedir(dp) == -1)
+		perror("dump_devices(): Failed to close directory");
+
+	return ret;
 }
 
 int main(int argc, char **argv)
@@ -163,7 +185,5 @@ int main(int argc, char **argv)
 		exit(1);
 	}
 
-	dump_devices();
-
-	return 0;
+	return dump_devices();
 }
#41 lore raw

PATCH 23/32 tools:iio:iio_utils: add missing documentation

In reply to Hartmut Knaack

Fully document public functions and elements.

---
 tools/iio/iio_utils.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++----
 tools/iio/iio_utils.h |  5 ++-
 2 files changed, 83 insertions(+), 7 deletions(-)
tools/iio/iio_utils.c +79 -6
index e1828d0..5d5831d 100644
@@ -29,6 +29,8 @@ static char * const iio_direction[] = {
  * iioutils_break_up_name() - extract generic name from full channel name
  * @full_name: the full channel name
  * @generic_name: the output generic channel name
+ *
+ * Returns 0 on success, or a negative error code if string extraction failed.
  **/
 int iioutils_break_up_name(const char *full_name,
 				  char **generic_name)
@@ -76,11 +78,15 @@ int iioutils_break_up_name(const char *full_name,
  * iioutils_get_type() - find and process _type attribute data
  * @is_signed: output whether channel is signed
  * @bytes: output how many bytes the channel storage occupies
+ * @bits_used: output number of valid bits of data
+ * @shift: output amount of bits to shift right data before applying bit mask
  * @mask: output a bit mask for the raw data
- * @be: big endian
- * @device_dir: the iio device directory
+ * @be: output if data in big endian
+ * @device_dir: the IIO device directory
  * @name: the channel name
  * @generic_name: the channel type name
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
  **/
 int iioutils_get_type(unsigned *is_signed,
 			     unsigned *bytes,
@@ -200,6 +206,16 @@ error_ret:
 	return ret;
 }
 
+/**
+ * iioutils_get_param_float() - read a float value from a channel parameter
+ * @output: output the float value
+ * @param_name: the parameter name to read
+ * @device_dir: the IIO device directory in sysfs
+ * @name: the channel name
+ * @generic_name: the channel type name
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int iioutils_get_param_float(float *output,
 				    const char *param_name,
 				    const char *device_dir,
@@ -266,8 +282,9 @@ error_ret:
 }
 
 /**
- * bsort_channel_array_by_index() - reorder so that the array is in index order
- *
+ * bsort_channel_array_by_index() - sort the array in index order
+ * @ci_array: the iio_channel_info array to be sorted
+ * @cnt: the amount of array elements
  **/
 
 void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
@@ -289,7 +306,10 @@ void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
 /**
  * build_channel_array() - function to figure out what channels are present
  * @device_dir: the IIO device directory in sysfs
- * @
+ * @ci_array: output the resulting array of iio_channel_info
+ * @counter: output the amount of array elements
+ *
+ * Returns 0 on success, otherwise a negative error code.
  **/
 int build_channel_array(const char *device_dir,
 			      struct iio_channel_info **ci_array,
@@ -525,8 +545,10 @@ int calc_digits(int num)
 /**
  * find_type_by_name() - function to match top level types by name
  * @name: top level type instance name
- * @type: the type of top level instance being sort
+ * @type: the type of top level instance being searched
  *
+ * Returns the device number of a matched IIO device on success, otherwise a
+ * negative error code.
  * Typical types this is used for are device and trigger.
  **/
 int find_type_by_name(const char *name, const char *type)
@@ -684,11 +706,28 @@ error_free:
 	return ret;
 }
 
+/**
+ * write_sysfs_int() - write an integer value to a sysfs file
+ * @filename: name of the file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: integer value to write to file
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_int(char *filename, char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 0);
 }
 
+/**
+ * write_sysfs_int_and_verify() - write an integer value to a sysfs file
+ *				  and verify
+ * @filename: name of the file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: integer value to write to file
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_int_and_verify(char *filename, char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 1);
@@ -770,17 +809,35 @@ error_free:
  * @filename: name of file to write to
  * @basedir: the sysfs directory in which the file is to be found
  * @val: the string to write
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
  **/
 int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 1);
 }
 
+/**
+ * write_sysfs_string() - write string to a sysfs file
+ * @filename: name of file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: the string to write
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_string(char *filename, char *basedir, char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 0);
 }
 
+/**
+ * read_sysfs_posint() - read an integer value from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ *
+ * Returns the read integer value >= 0 on success, otherwise a negative error
+ * code.
+ **/
 int read_sysfs_posint(char *filename, char *basedir)
 {
 	int ret;
@@ -817,6 +874,14 @@ error_free:
 	return ret;
 }
 
+/**
+ * read_sysfs_float() - read a float value from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: output the read float value
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int read_sysfs_float(char *filename, char *basedir, float *val)
 {
 	int ret = 0;
@@ -853,6 +918,14 @@ error_free:
 	return ret;
 }
 
+/**
+ * read_sysfs_string() - read a string from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ * @str: output the read string
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int read_sysfs_string(const char *filename, const char *basedir, char *str)
 {
 	int ret = 0;
tools/iio/iio_utils.h +4 -1
index 1bc837b..e5eb46a 100644
@@ -28,9 +28,12 @@ extern const char *iio_dir;
  * @offset: offset to be applied for conversion to si units
  * @index: the channel index in the buffer output
  * @bytes: number of bytes occupied in buffer output
+ * @bits_used: number of valid bits of data
+ * @shift: amount of bits to shift right data before applying bit mask
  * @mask: a bit mask for the raw output
+ * @be: flag if data is big endian
  * @is_signed: is the raw value stored signed
- * @enabled: is this channel enabled
+ * @location: data offset for this channel inside the buffer (in bytes)
  **/
 struct iio_channel_info {
 	char *name;
Signature · 1 line
2.3.6
maintainer Applied
#42 lore raw

Re: [PATCH 23/32] tools:iio:iio_utils: add missing documentation

Quoted 3 lines · Hartmut Knaack

Fully document public functions and elements.

Applied.

Quoted 203 lines
---
 tools/iio/iio_utils.c | 85 +++++++++++++++++++++++++++++++++++++++++++++++----
 tools/iio/iio_utils.h |  5 ++-
 2 files changed, 83 insertions(+), 7 deletions(-)
tools/iio/iio_utils.c +79 -6
index e1828d0..5d5831d 100644
@@ -29,6 +29,8 @@ static char * const iio_direction[] = {
  * iioutils_break_up_name() - extract generic name from full channel name
  * @full_name: the full channel name
  * @generic_name: the output generic channel name
+ *
+ * Returns 0 on success, or a negative error code if string extraction failed.
  **/
 int iioutils_break_up_name(const char *full_name,
 				  char **generic_name)
@@ -76,11 +78,15 @@ int iioutils_break_up_name(const char *full_name,
  * iioutils_get_type() - find and process _type attribute data
  * @is_signed: output whether channel is signed
  * @bytes: output how many bytes the channel storage occupies
+ * @bits_used: output number of valid bits of data
+ * @shift: output amount of bits to shift right data before applying bit mask
  * @mask: output a bit mask for the raw data
- * @be: big endian
- * @device_dir: the iio device directory
+ * @be: output if data in big endian
+ * @device_dir: the IIO device directory
  * @name: the channel name
  * @generic_name: the channel type name
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
  **/
 int iioutils_get_type(unsigned *is_signed,
 			     unsigned *bytes,
@@ -200,6 +206,16 @@ error_ret:
 	return ret;
 }
 
+/**
+ * iioutils_get_param_float() - read a float value from a channel parameter
+ * @output: output the float value
+ * @param_name: the parameter name to read
+ * @device_dir: the IIO device directory in sysfs
+ * @name: the channel name
+ * @generic_name: the channel type name
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int iioutils_get_param_float(float *output,
 				    const char *param_name,
 				    const char *device_dir,
@@ -266,8 +282,9 @@ error_ret:
 }
 
 /**
- * bsort_channel_array_by_index() - reorder so that the array is in index order
- *
+ * bsort_channel_array_by_index() - sort the array in index order
+ * @ci_array: the iio_channel_info array to be sorted
+ * @cnt: the amount of array elements
  **/
 
 void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
@@ -289,7 +306,10 @@ void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
 /**
  * build_channel_array() - function to figure out what channels are present
  * @device_dir: the IIO device directory in sysfs
- * @
+ * @ci_array: output the resulting array of iio_channel_info
+ * @counter: output the amount of array elements
+ *
+ * Returns 0 on success, otherwise a negative error code.
  **/
 int build_channel_array(const char *device_dir,
 			      struct iio_channel_info **ci_array,
@@ -525,8 +545,10 @@ int calc_digits(int num)
 /**
  * find_type_by_name() - function to match top level types by name
  * @name: top level type instance name
- * @type: the type of top level instance being sort
+ * @type: the type of top level instance being searched
  *
+ * Returns the device number of a matched IIO device on success, otherwise a
+ * negative error code.
  * Typical types this is used for are device and trigger.
  **/
 int find_type_by_name(const char *name, const char *type)
@@ -684,11 +706,28 @@ error_free:
 	return ret;
 }
 
+/**
+ * write_sysfs_int() - write an integer value to a sysfs file
+ * @filename: name of the file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: integer value to write to file
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_int(char *filename, char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 0);
 }
 
+/**
+ * write_sysfs_int_and_verify() - write an integer value to a sysfs file
+ *				  and verify
+ * @filename: name of the file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: integer value to write to file
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_int_and_verify(char *filename, char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 1);
@@ -770,17 +809,35 @@ error_free:
  * @filename: name of file to write to
  * @basedir: the sysfs directory in which the file is to be found
  * @val: the string to write
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
  **/
 int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 1);
 }
 
+/**
+ * write_sysfs_string() - write string to a sysfs file
+ * @filename: name of file to write to
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: the string to write
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int write_sysfs_string(char *filename, char *basedir, char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 0);
 }
 
+/**
+ * read_sysfs_posint() - read an integer value from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ *
+ * Returns the read integer value >= 0 on success, otherwise a negative error
+ * code.
+ **/
 int read_sysfs_posint(char *filename, char *basedir)
 {
 	int ret;
@@ -817,6 +874,14 @@ error_free:
 	return ret;
 }
 
+/**
+ * read_sysfs_float() - read a float value from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ * @val: output the read float value
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int read_sysfs_float(char *filename, char *basedir, float *val)
 {
 	int ret = 0;
@@ -853,6 +918,14 @@ error_free:
 	return ret;
 }
 
+/**
+ * read_sysfs_string() - read a string from file
+ * @filename: name of file to read from
+ * @basedir: the sysfs directory in which the file is to be found
+ * @str: output the read string
+ *
+ * Returns a value >= 0 on success, otherwise a negative error code.
+ **/
 int read_sysfs_string(const char *filename, const char *basedir, char *str)
 {
 	int ret = 0;
tools/iio/iio_utils.h +4 -1
index 1bc837b..e5eb46a 100644
@@ -28,9 +28,12 @@ extern const char *iio_dir;
  * @offset: offset to be applied for conversion to si units
  * @index: the channel index in the buffer output
  * @bytes: number of bytes occupied in buffer output
+ * @bits_used: number of valid bits of data
+ * @shift: amount of bits to shift right data before applying bit mask
  * @mask: a bit mask for the raw output
+ * @be: flag if data is big endian
  * @is_signed: is the raw value stored signed
- * @enabled: is this channel enabled
+ * @location: data offset for this channel inside the buffer (in bytes)
  **/
 struct iio_channel_info {
 	char *name;
#43 lore raw

PATCH 24/32 tools:iio: return values directly

In reply to Hartmut Knaack

Return directly, if no common cleanup is required.

---
 tools/iio/generic_buffer.c    |  5 ++---
 tools/iio/iio_event_monitor.c |  5 ++---
 tools/iio/iio_utils.c         | 27 ++++++++++++---------------
 3 files changed, 16 insertions(+), 21 deletions(-)
tools/iio/generic_buffer.c +2 -3
index 2f48843..f17b168 100644
@@ -238,8 +238,7 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num < 0) {
 		printf("Failed to find the %s\n", device_name);
-		ret = dev_num;
-		goto error_ret;
+		return dev_num;
 	}
 	printf("iio device number being used is %d\n", dev_num);
 
@@ -410,6 +409,6 @@ error_free_triggername:
 		free(trigger_name);
 error_free_dev_dir_name:
 	free(dev_dir_name);
-error_ret:
+
 	return ret;
 }
tools/iio/iio_event_monitor.c +2 -3
index ecee8ac..17c27bf 100644
@@ -258,8 +258,7 @@ int main(int argc, char **argv)
 			device_name, dev_num);
 		ret = asprintf(&chrdev_name, "/dev/iio:device%d", dev_num);
 		if (ret < 0) {
-			ret = -ENOMEM;
-			goto error_ret;
+			return -ENOMEM;
 		}
 	} else {
 		/* If we can't find a IIO device by name assume device_name is a
@@ -317,6 +316,6 @@ int main(int argc, char **argv)
 
 error_free_chrdev_name:
 	free(chrdev_name);
-error_ret:
+
 	return ret;
 }
tools/iio/iio_utils.c +12 -15
index 5d5831d..0524725 100644
@@ -107,10 +107,9 @@ int iioutils_get_type(unsigned *is_signed,
 	const struct dirent *ent;
 
 	ret = asprintf(&scan_el_dir, FORMAT_SCAN_ELEMENTS_DIR, device_dir);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	ret = asprintf(&builtname, FORMAT_TYPE_FILE, name);
 	if (ret < 0) {
 		ret = -ENOMEM;
@@ -202,7 +201,7 @@ error_free_builtname:
 	free(builtname);
 error_free_scan_el_dir:
 	free(scan_el_dir);
-error_ret:
+
 	return ret;
 }
 
@@ -230,10 +229,9 @@ int iioutils_get_param_float(float *output,
 	const struct dirent *ent;
 
 	ret = asprintf(&builtname, "%s_%s", name, param_name);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	ret = asprintf(&builtname_generic,
 		       "%s_%s", generic_name, param_name);
 	if (ret < 0) {
@@ -277,7 +275,7 @@ error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
 	free(builtname);
-error_ret:
+
 	return ret;
 }
 
@@ -326,10 +324,9 @@ int build_channel_array(const char *device_dir,
 
 	*counter = 0;
 	ret = asprintf(&scan_el_dir, FORMAT_SCAN_ELEMENTS_DIR, device_dir);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	dp = opendir(scan_el_dir);
 	if (dp == NULL) {
 		ret = -errno;
@@ -526,7 +523,7 @@ error_close_dir:
 
 error_free_name:
 	free(scan_el_dir);
-error_ret:
+
 	return ret;
 }
 
Signature · 1 line
2.3.6
maintainer Applied
#44 lore raw

Re: [PATCH 24/32] tools:iio: return values directly

Quoted 3 lines · Hartmut Knaack

Return directly, if no common cleanup is required.

Applied.

Quoted 124 lines
---
 tools/iio/generic_buffer.c    |  5 ++---
 tools/iio/iio_event_monitor.c |  5 ++---
 tools/iio/iio_utils.c         | 27 ++++++++++++---------------
 3 files changed, 16 insertions(+), 21 deletions(-)
tools/iio/generic_buffer.c +2 -3
index 2f48843..f17b168 100644
@@ -238,8 +238,7 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num < 0) {
 		printf("Failed to find the %s\n", device_name);
-		ret = dev_num;
-		goto error_ret;
+		return dev_num;
 	}
 	printf("iio device number being used is %d\n", dev_num);
 
@@ -410,6 +409,6 @@ error_free_triggername:
 		free(trigger_name);
 error_free_dev_dir_name:
 	free(dev_dir_name);
-error_ret:
+
 	return ret;
 }
tools/iio/iio_event_monitor.c +2 -3
index ecee8ac..17c27bf 100644
@@ -258,8 +258,7 @@ int main(int argc, char **argv)
 			device_name, dev_num);
 		ret = asprintf(&chrdev_name, "/dev/iio:device%d", dev_num);
 		if (ret < 0) {
-			ret = -ENOMEM;
-			goto error_ret;
+			return -ENOMEM;
 		}
 	} else {
 		/* If we can't find a IIO device by name assume device_name is a
@@ -317,6 +316,6 @@ int main(int argc, char **argv)
 
 error_free_chrdev_name:
 	free(chrdev_name);
-error_ret:
+
 	return ret;
 }
tools/iio/iio_utils.c +12 -15
index 5d5831d..0524725 100644
@@ -107,10 +107,9 @@ int iioutils_get_type(unsigned *is_signed,
 	const struct dirent *ent;
 
 	ret = asprintf(&scan_el_dir, FORMAT_SCAN_ELEMENTS_DIR, device_dir);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	ret = asprintf(&builtname, FORMAT_TYPE_FILE, name);
 	if (ret < 0) {
 		ret = -ENOMEM;
@@ -202,7 +201,7 @@ error_free_builtname:
 	free(builtname);
 error_free_scan_el_dir:
 	free(scan_el_dir);
-error_ret:
+
 	return ret;
 }
 
@@ -230,10 +229,9 @@ int iioutils_get_param_float(float *output,
 	const struct dirent *ent;
 
 	ret = asprintf(&builtname, "%s_%s", name, param_name);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	ret = asprintf(&builtname_generic,
 		       "%s_%s", generic_name, param_name);
 	if (ret < 0) {
@@ -277,7 +275,7 @@ error_free_builtname_generic:
 	free(builtname_generic);
 error_free_builtname:
 	free(builtname);
-error_ret:
+
 	return ret;
 }
 
@@ -326,10 +324,9 @@ int build_channel_array(const char *device_dir,
 
 	*counter = 0;
 	ret = asprintf(&scan_el_dir, FORMAT_SCAN_ELEMENTS_DIR, device_dir);
-	if (ret < 0) {
-		ret = -ENOMEM;
-		goto error_ret;
-	}
+	if (ret < 0)
+		return -ENOMEM;
+
 	dp = opendir(scan_el_dir);
 	if (dp == NULL) {
 		ret = -errno;
@@ -526,7 +523,7 @@ error_close_dir:
 
 error_free_name:
 	free(scan_el_dir);
-error_ret:
+
 	return ret;
 }
 
#45 lore raw

PATCH 25/32 tools:iio:iio_event_monitor: refactor events output

In reply to Hartmut Knaack

Refactor the code in print_event() to reduce code duplication and better reflect that the type is output unconditionally, as well as cascade the dependency of the diff-channel. Saves a few lines of code, as well.

---
 tools/iio/iio_event_monitor.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
tools/iio/iio_event_monitor.c +10 -14
index 17c27bf..8e682a7 100644
@@ -213,23 +213,19 @@ static void print_event(struct iio_event_data *event)
 		return;
 	}
 
-	printf("Event: time: %lld, ", event->timestamp);
+	printf("Event: time: %lld, type: %s", event->timestamp,
+	       iio_chan_type_name_spec[type]);
 
-	if (mod != IIO_NO_MOD) {
-		printf("type: %s(%s), ",
-			iio_chan_type_name_spec[type],
-			iio_modifier_names[mod]);
-	} else {
-		printf("type: %s, ",
-			iio_chan_type_name_spec[type]);
-	}
+	if (mod != IIO_NO_MOD)
+		printf("(%s)", iio_modifier_names[mod]);
 
-	if (diff && chan >= 0 && chan2 >= 0)
-		printf("channel: %d-%d, ", chan, chan2);
-	else if (chan >= 0)
-		printf("channel: %d, ", chan);
+	if (chan >= 0) {
+		printf(", channel: %d", chan);
+		if (diff && chan2 >= 0)
+			printf("-%d", chan2);
+	}
 
-	printf("evtype: %s", iio_ev_type_text[ev_type]);
+	printf(", evtype: %s", iio_ev_type_text[ev_type]);
 
 	if (dir != IIO_EV_DIR_NONE)
 		printf(", direction: %s", iio_ev_dir_text[dir]);
Signature · 1 line
2.3.6
maintainer Applied
#46 lore raw

Re: [PATCH 25/32] tools:iio:iio_event_monitor: refactor events output

Quoted 5 lines · Hartmut Knaack

Refactor the code in print_event() to reduce code duplication and better reflect that the type is output unconditionally, as well as cascade the dependency of the diff-channel. Saves a few lines of code, as well.

Applied.

Quoted 43 lines
---
 tools/iio/iio_event_monitor.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
tools/iio/iio_event_monitor.c +10 -14
index 17c27bf..8e682a7 100644
@@ -213,23 +213,19 @@ static void print_event(struct iio_event_data *event)
 		return;
 	}
 
-	printf("Event: time: %lld, ", event->timestamp);
+	printf("Event: time: %lld, type: %s", event->timestamp,
+	       iio_chan_type_name_spec[type]);
 
-	if (mod != IIO_NO_MOD) {
-		printf("type: %s(%s), ",
-			iio_chan_type_name_spec[type],
-			iio_modifier_names[mod]);
-	} else {
-		printf("type: %s, ",
-			iio_chan_type_name_spec[type]);
-	}
+	if (mod != IIO_NO_MOD)
+		printf("(%s)", iio_modifier_names[mod]);
 
-	if (diff && chan >= 0 && chan2 >= 0)
-		printf("channel: %d-%d, ", chan, chan2);
-	else if (chan >= 0)
-		printf("channel: %d, ", chan);
+	if (chan >= 0) {
+		printf(", channel: %d", chan);
+		if (diff && chan2 >= 0)
+			printf("-%d", chan2);
+	}
 
-	printf("evtype: %s", iio_ev_type_text[ev_type]);
+	printf(", evtype: %s", iio_ev_type_text[ev_type]);
 
 	if (dir != IIO_EV_DIR_NONE)
 		printf(", direction: %s", iio_ev_dir_text[dir]);
#47 lore raw

PATCH 26/32 tools:iio:iio_utils: refactor assignment of is_signed

In reply to Hartmut Knaack

Change the assignment of *is_signed in iioutils_get_type() to a one-liner, as already done with *be.

---
 tools/iio/iio_utils.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
tools/iio/iio_utils.c +1 -4
index 0524725..c83f4df 100644
@@ -168,10 +168,7 @@ int iioutils_get_type(unsigned *is_signed,
 				*mask = ~0;
 			else
 				*mask = (1 << *bits_used) - 1;
-			if (signchar == 's')
-				*is_signed = 1;
-			else
-				*is_signed = 0;
+			*is_signed = (signchar == 's');
 			if (fclose(sysfsfp)) {
 				ret = -errno;
 				printf("Failed to close %s\n", filename);
Signature · 1 line
2.3.6
maintainer Applied
#48 lore raw

Re: [PATCH 26/32] tools:iio:iio_utils: refactor assignment of is_signed

Quoted 4 lines · Hartmut Knaack

Change the assignment of *is_signed in iioutils_get_type() to a one-liner, as already done with *be.

Applied.

Quoted 21 lines
---
 tools/iio/iio_utils.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
tools/iio/iio_utils.c +1 -4
index 0524725..c83f4df 100644
@@ -168,10 +168,7 @@ int iioutils_get_type(unsigned *is_signed,
 				*mask = ~0;
 			else
 				*mask = (1 << *bits_used) - 1;
-			if (signchar == 's')
-				*is_signed = 1;
-			else
-				*is_signed = 0;
+			*is_signed = (signchar == 's');
 			if (fclose(sysfsfp)) {
 				ret = -errno;
 				printf("Failed to close %s\n", filename);
#49 lore raw

PATCH 27/32 tools:iio:iio_utils: move up reset of sysfsfp

In reply to Hartmut Knaack

In iioutils_get_type() it is logically better fitting to have sysfsfp assigned zero right after closing it.

---
 tools/iio/iio_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/iio_utils.c +1 -1
index c83f4df..7c0abb3 100644
@@ -175,10 +175,10 @@ int iioutils_get_type(unsigned *is_signed,
 				goto error_free_filename;
 			}
 
+			sysfsfp = 0;
 			free(filename);
 
 			filename = 0;
-			sysfsfp = 0;
 		}
 error_close_sysfsfp:
 	if (sysfsfp)
Signature · 1 line
2.3.6
maintainer Applied
#50 lore raw

Re: [PATCH 27/32] tools:iio:iio_utils: move up reset of sysfsfp

Quoted 4 lines · Hartmut Knaack

In iioutils_get_type() it is logically better fitting to have sysfsfp assigned zero right after closing it.

Applied.

Quoted 21 lines
---
 tools/iio/iio_utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/iio/iio_utils.c +1 -1
index c83f4df..7c0abb3 100644
@@ -175,10 +175,10 @@ int iioutils_get_type(unsigned *is_signed,
 				goto error_free_filename;
 			}
 
+			sysfsfp = 0;
 			free(filename);
 
 			filename = 0;
-			sysfsfp = 0;
 		}
 error_close_sysfsfp:
 	if (sysfsfp)
#51 lore raw

PATCH 28/32 tools:iio:iio_utils: initialize count during declaration

In reply to Hartmut Knaack

In build_channel_array(), count can be initialized already during variable declaration.

---
 tools/iio/iio_utils.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
tools/iio/iio_utils.c +1 -2
index 7c0abb3..dfee1a3 100644
@@ -312,7 +312,7 @@ int build_channel_array(const char *device_dir,
 {
 	DIR *dp;
 	FILE *sysfsfp;
-	int count, i;
+	int count = 0, i;
 	struct iio_channel_info *current;
 	int ret;
 	const struct dirent *ent;
@@ -370,7 +370,6 @@ int build_channel_array(const char *device_dir,
 		goto error_close_dir;
 	}
 	seekdir(dp, 0);
-	count = 0;
 	while (ent = readdir(dp), ent != NULL) {
 		if (strcmp(ent->d_name + strlen(ent->d_name) - strlen("_en"),
 			   "_en") == 0) {
Signature · 1 line
2.3.6
maintainer Applied
#52 lore raw

Re: [PATCH 28/32] tools:iio:iio_utils: initialize count during declaration

Quoted 4 lines · Hartmut Knaack

In build_channel_array(), count can be initialized already during variable declaration.

Applied.

Quoted 26 lines
---
 tools/iio/iio_utils.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
tools/iio/iio_utils.c +1 -2
index 7c0abb3..dfee1a3 100644
@@ -312,7 +312,7 @@ int build_channel_array(const char *device_dir,
 {
 	DIR *dp;
 	FILE *sysfsfp;
-	int count, i;
+	int count = 0, i;
 	struct iio_channel_info *current;
 	int ret;
 	const struct dirent *ent;
@@ -370,7 +370,6 @@ int build_channel_array(const char *device_dir,
 		goto error_close_dir;
 	}
 	seekdir(dp, 0);
-	count = 0;
 	while (ent = readdir(dp), ent != NULL) {
 		if (strcmp(ent->d_name + strlen(ent->d_name) - strlen("_en"),
 			   "_en") == 0) {
#53 lore raw

PATCH 29/32 tools:iio: rework program parameters

In reply to Hartmut Knaack

In generic_buffer.c: sort program parameters alphabetically and provide
usage information
In lsiio.c: drop unused parameters

---
 tools/iio/generic_buffer.c | 55 ++++++++++++++++++++++++++++++----------------
 tools/iio/lsiio.c          |  6 ++---
 2 files changed, 38 insertions(+), 23 deletions(-)
tools/iio/generic_buffer.c +36 -19
index f17b168..c546852 100644
@@ -168,6 +168,19 @@ void process_scan(char *data,
 	printf("\n");
 }
 
+void print_usage(void)
+{
+	printf("Usage: generic_buffer [options]...\n"
+	       "Capture, convert and output data from IIO device buffer\n"
+	       "  -c <n>     Do n conversions\n"
+	       "  -e         Disable wait for event (new data)\n"
+	       "  -g         Use trigger-less mode\n"
+	       "  -l <n>     Set buffer length to n samples\n"
+	       "  -n <name>  Set device name (mandatory)\n"
+	       "  -t <name>  Set trigger name\n"
+	       "  -w <n>     Set delay between reads in us (event-less mode)\n");
+}
+
 int main(int argc, char **argv)
 {
 	unsigned long num_loops = 2;
@@ -193,29 +206,19 @@ int main(int argc, char **argv)
 
 	struct iio_channel_info *channels;
 
-	while ((c = getopt(argc, argv, "l:w:c:et:n:g")) != -1) {
+	while ((c = getopt(argc, argv, "c:egl:n:t:w:")) != -1) {
 		switch (c) {
-		case 'n':
-			device_name = optarg;
-			break;
-		case 't':
-			trigger_name = optarg;
-			datardytrigger = 0;
-			break;
-		case 'e':
-			noevents = 1;
-			break;
 		case 'c':
 			errno = 0;
 			num_loops = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
 			break;
-		case 'w':
-			errno = 0;
-			timedelay = strtoul(optarg, &dummy, 10);
-			if (errno)
-				return -errno;
+		case 'e':
+			noevents = 1;
+			break;
+		case 'g':
+			notrigger = 1;
 			break;
 		case 'l':
 			errno = 0;
@@ -223,16 +226,30 @@ int main(int argc, char **argv)
 			if (errno)
 				return -errno;
 			break;
-		case 'g':
-			notrigger = 1;
+		case 'n':
+			device_name = optarg;
+			break;
+		case 't':
+			trigger_name = optarg;
+			datardytrigger = 0;
+			break;
+		case 'w':
+			errno = 0;
+			timedelay = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
 			break;
 		case '?':
+			print_usage();
 			return -1;
 		}
 	}
 
-	if (device_name == NULL)
+	if (device_name == NULL) {
+		printf("Device name not set\n");
+		print_usage();
 		return -1;
+	}
 
 	/* Find the device requested */
 	dev_num = find_type_by_name(device_name, "iio:device");
tools/iio/lsiio.c +2 -4
index b1089ad..b59ee17 100644
@@ -164,7 +164,7 @@ int main(int argc, char **argv)
 {
 	int c, err = 0;
 
-	while ((c = getopt(argc, argv, "d:D:v")) != EOF) {
+	while ((c = getopt(argc, argv, "v")) != EOF) {
 		switch (c) {
 		case 'v':
 			verblevel++;
@@ -179,9 +179,7 @@ int main(int argc, char **argv)
 	if (err || argc > optind) {
 		fprintf(stderr, "Usage: lsiio [options]...\n"
 			"List industrial I/O devices\n"
-			"  -v, --verbose\n"
-			"      Increase verbosity (may be given multiple times)\n"
-			);
+			"  -v  Increase verbosity (may be given multiple times)\n");
 		exit(1);
 	}
 
Signature · 1 line
2.3.6
maintainer Applied
#54 lore raw

Re: [PATCH 29/32] tools:iio: rework program parameters

Quoted 5 lines · Hartmut Knaack

In generic_buffer.c: sort program parameters alphabetically and provide
usage information
In lsiio.c: drop unused parameters

Applied.

Quoted 124 lines
---
 tools/iio/generic_buffer.c | 55 ++++++++++++++++++++++++++++++----------------
 tools/iio/lsiio.c          |  6 ++---
 2 files changed, 38 insertions(+), 23 deletions(-)
tools/iio/generic_buffer.c +36 -19
index f17b168..c546852 100644
@@ -168,6 +168,19 @@ void process_scan(char *data,
 	printf("\n");
 }
 
+void print_usage(void)
+{
+	printf("Usage: generic_buffer [options]...\n"
+	       "Capture, convert and output data from IIO device buffer\n"
+	       "  -c <n>     Do n conversions\n"
+	       "  -e         Disable wait for event (new data)\n"
+	       "  -g         Use trigger-less mode\n"
+	       "  -l <n>     Set buffer length to n samples\n"
+	       "  -n <name>  Set device name (mandatory)\n"
+	       "  -t <name>  Set trigger name\n"
+	       "  -w <n>     Set delay between reads in us (event-less mode)\n");
+}
+
 int main(int argc, char **argv)
 {
 	unsigned long num_loops = 2;
@@ -193,29 +206,19 @@ int main(int argc, char **argv)
 
 	struct iio_channel_info *channels;
 
-	while ((c = getopt(argc, argv, "l:w:c:et:n:g")) != -1) {
+	while ((c = getopt(argc, argv, "c:egl:n:t:w:")) != -1) {
 		switch (c) {
-		case 'n':
-			device_name = optarg;
-			break;
-		case 't':
-			trigger_name = optarg;
-			datardytrigger = 0;
-			break;
-		case 'e':
-			noevents = 1;
-			break;
 		case 'c':
 			errno = 0;
 			num_loops = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
 			break;
-		case 'w':
-			errno = 0;
-			timedelay = strtoul(optarg, &dummy, 10);
-			if (errno)
-				return -errno;
+		case 'e':
+			noevents = 1;
+			break;
+		case 'g':
+			notrigger = 1;
 			break;
 		case 'l':
 			errno = 0;
@@ -223,16 +226,30 @@ int main(int argc, char **argv)
 			if (errno)
 				return -errno;
 			break;
-		case 'g':
-			notrigger = 1;
+		case 'n':
+			device_name = optarg;
+			break;
+		case 't':
+			trigger_name = optarg;
+			datardytrigger = 0;
+			break;
+		case 'w':
+			errno = 0;
+			timedelay = strtoul(optarg, &dummy, 10);
+			if (errno)
+				return -errno;
 			break;
 		case '?':
+			print_usage();
 			return -1;
 		}
 	}
 
-	if (device_name == NULL)
+	if (device_name == NULL) {
+		printf("Device name not set\n");
+		print_usage();
 		return -1;
+	}
 
 	/* Find the device requested */
 	dev_num = find_type_by_name(device_name, "iio:device");
tools/iio/lsiio.c +2 -4
index b1089ad..b59ee17 100644
@@ -164,7 +164,7 @@ int main(int argc, char **argv)
 {
 	int c, err = 0;
 
-	while ((c = getopt(argc, argv, "d:D:v")) != EOF) {
+	while ((c = getopt(argc, argv, "v")) != EOF) {
 		switch (c) {
 		case 'v':
 			verblevel++;
@@ -179,9 +179,7 @@ int main(int argc, char **argv)
 	if (err || argc > optind) {
 		fprintf(stderr, "Usage: lsiio [options]...\n"
 			"List industrial I/O devices\n"
-			"  -v, --verbose\n"
-			"      Increase verbosity (may be given multiple times)\n"
-			);
+			"  -v  Increase verbosity (may be given multiple times)\n");
 		exit(1);
 	}
 
maintainer
#55 lore raw

Re: [PATCH 20/32] tools:iio:iio_event_monitor: check if event is fully read

In reply to Hartmut Knaack
Quoted 3 lines · Hartmut Knaack

Check that the read event is of the expected size.

Is this a meaningful thing to do? Something has gone horribly wrong if this occurs.. Perhaps just dropping out entirely is a better bet?

Quoted 21 lines
---
 tools/iio/iio_event_monitor.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/iio/iio_event_monitor.c +5 -0
index 1316527..ecee8ac 100644
@@ -304,6 +304,11 @@ int main(int argc, char **argv)
 			}
 		}
 
+		if (ret != sizeof(event)) {
+			printf("Reading event failed, trying again\n");
+			continue;
+		}
+
 		print_event(&event);
 	}
 
#56 lore raw

Re: [PATCH 20/32] tools:iio:iio_event_monitor: check if event is fully read

Jonathan Cameron schrieb am 31.05.2015 um 20:36:

Quoted 7 lines · Hartmut Knaack
Quoted 3 lines

Check that the read event is of the expected size.

Is this a meaningful thing to do? Something has gone horribly wrong if this occurs.. Perhaps just dropping out entirely is a better bet?

Well, my main intention was just to prevent passing garbage data to print_event(). But if you put it this way, then aborting seems more reasonable. Will change it. Thanks,

Hartmut

Quoted 27 lines
Quoted 11 lines
---
 tools/iio/iio_event_monitor.c | 5 +++++
 1 file changed, 5 insertions(+)
tools/iio/iio_event_monitor.c +0 -0
index 1316527..ecee8ac 100644
@@ -304,6 +304,11 @@ int main(int argc, char **argv)
 			}

tools/iio/iio_event_monitor.c int main(int argc, char **argv) · line 314

Quoted 10 lines
tools/iio/iio_event_monitor.c +5 -0
 		}
 
+		if (ret != sizeof(event)) {
+			printf("Reading event failed, trying again\n");
+			continue;
+		}
+
 		print_event(&event);
 	}
 

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#57 lore raw

PATCH 30/32 tools:iio:iio_utils: pass strings as const

Mark strings, which are not supposed to be changed (basedir, filename, value), as const in function parameters.

---
 tools/iio/iio_utils.c | 22 +++++++++++++---------
 tools/iio/iio_utils.h | 15 +++++++++------
 2 files changed, 22 insertions(+), 15 deletions(-)
tools/iio/iio_utils.c +13 -9
index dfee1a3..ec9ab7f 100644
@@ -634,7 +634,8 @@ error_close_dir:
 	return ret;
 }
 
-static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
+static int _write_sysfs_int(const char *filename, const char *basedir, int val,
+			    int verify)
 {
 	int ret = 0;
 	FILE *sysfsfp;
@@ -707,7 +708,7 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_int(char *filename, char *basedir, int val)
+int write_sysfs_int(const char *filename, const char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 0);
 }
@@ -721,13 +722,14 @@ int write_sysfs_int(char *filename, char *basedir, int val)
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_int_and_verify(char *filename, char *basedir, int val)
+int write_sysfs_int_and_verify(const char *filename, const char *basedir,
+			       int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 1);
 }
 
-static int _write_sysfs_string(char *filename, char *basedir, char *val,
-			       int verify)
+static int _write_sysfs_string(const char *filename, const char *basedir,
+			       const char *val, int verify)
 {
 	int ret = 0;
 	FILE  *sysfsfp;
@@ -805,7 +807,8 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
+int write_sysfs_string_and_verify(const char *filename, const char *basedir,
+				  const char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 1);
 }
@@ -818,7 +821,8 @@ int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_string(char *filename, char *basedir, char *val)
+int write_sysfs_string(const char *filename, const char *basedir,
+		       const char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 0);
 }
@@ -831,7 +835,7 @@ int write_sysfs_string(char *filename, char *basedir, char *val)
  * Returns the read integer value >= 0 on success, otherwise a negative error
  * code.
  **/
-int read_sysfs_posint(char *filename, char *basedir)
+int read_sysfs_posint(const char *filename, const char *basedir)
 {
 	int ret;
 	FILE  *sysfsfp;
@@ -875,7 +879,7 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int read_sysfs_float(char *filename, char *basedir, float *val)
+int read_sysfs_float(const char *filename, const char *basedir, float *val)
 {
 	int ret = 0;
 	FILE  *sysfsfp;
tools/iio/iio_utils.h +9 -6
index e5eb46a..379eed9 100644
@@ -63,12 +63,15 @@ void bsort_channel_array_by_index(struct iio_channel_info **ci_array, int cnt);
 int build_channel_array(const char *device_dir,
 						struct iio_channel_info **ci_array, int *counter);
 int find_type_by_name(const char *name, const char *type);
-int write_sysfs_int(char *filename, char *basedir, int val);
-int write_sysfs_int_and_verify(char *filename, char *basedir, int val);
-int write_sysfs_string_and_verify(char *filename, char *basedir, char *val);
-int write_sysfs_string(char *filename, char *basedir, char *val);
-int read_sysfs_posint(char *filename, char *basedir);
-int read_sysfs_float(char *filename, char *basedir, float *val);
+int write_sysfs_int(const char *filename, const char *basedir, int val);
+int write_sysfs_int_and_verify(const char *filename, const char *basedir,
+			       int val);
+int write_sysfs_string_and_verify(const char *filename, const char *basedir,
+				  const char *val);
+int write_sysfs_string(const char *filename, const char *basedir,
+		       const char *val);
+int read_sysfs_posint(const char *filename, const char *basedir);
+int read_sysfs_float(const char *filename, const char *basedir, float *val);
 int read_sysfs_string(const char *filename, const char *basedir, char *str);
 
 #endif /* _IIO_UTILS_H_ */
Signature · 1 line
2.3.6
#58 lore raw

PATCH 31/32 tools:iio: adjust coding style

Fix various coding style issues, including:
* have spaces around operators
* indentation
* consolidate parameters in same line
* required braces
* adjust/drop comments
* multiline comment style
* delete unnecessary empty lines
* add empty lines to visualize logial code blocks
* typos

---
 tools/iio/generic_buffer.c    |  46 +++++++++--------
 tools/iio/iio_event_monitor.c |  13 +++--
 tools/iio/iio_utils.c         | 111 +++++++++++++++++++++++++-----------------
 tools/iio/iio_utils.h         |  15 +++---
 tools/iio/lsiio.c             |  15 +++---
 5 files changed, 118 insertions(+), 82 deletions(-)
tools/iio/generic_buffer.c +27 -19
index c546852..a4c14d0 100644
@@ -51,11 +51,13 @@ int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 		if (bytes % channels[i].bytes == 0)
 			channels[i].location = bytes;
 		else
-			channels[i].location = bytes - bytes%channels[i].bytes
-				+ channels[i].bytes;
+			channels[i].location = bytes - bytes % channels[i].bytes
+					       + channels[i].bytes;
+
 		bytes = channels[i].location + channels[i].bytes;
 		i++;
 	}
+
 	return bytes;
 }
 
@@ -136,9 +138,9 @@ void print8byte(uint64_t input, struct iio_channel_info *info)
 /**
  * process_scan() - print out the values in SI units
  * @data:		pointer to the start of the scan
- * @channels:		information about the channels. Note
- *  size_from_channelarray must have been called first to fill the
- *  location offsets.
+ * @channels:		information about the channels.
+ *			Note: size_from_channelarray must have been called first
+ *			      to fill the location offsets.
  * @num_channels:	number of channels
  **/
 void process_scan(char *data,
@@ -213,6 +215,7 @@ int main(int argc, char **argv)
 			num_loops = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
+
 			break;
 		case 'e':
 			noevents = 1;
@@ -225,6 +228,7 @@ int main(int argc, char **argv)
 			buf_len = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
+
 			break;
 		case 'n':
 			device_name = optarg;
@@ -257,6 +261,7 @@ int main(int argc, char **argv)
 		printf("Failed to find the %s\n", device_name);
 		return dev_num;
 	}
+
 	printf("iio device number being used is %d\n", dev_num);
 
 	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
@@ -285,9 +290,11 @@ int main(int argc, char **argv)
 			ret = trig_num;
 			goto error_free_triggername;
 		}
+
 		printf("iio trigger number being used is %d\n", trig_num);
-	} else
+	} else {
 		printf("trigger-less mode selected\n");
+	}
 
 	/*
 	 * Parse the files in scan_elements to identify what channels are
@@ -314,8 +321,10 @@ int main(int argc, char **argv)
 
 	if (!notrigger) {
 		printf("%s %s\n", dev_dir_name, trigger_name);
-		/* Set the device trigger to be the data ready trigger found
-		 * above */
+		/*
+		 * Set the device trigger to be the data ready trigger found
+		 * above
+		 */
 		ret = write_sysfs_string_and_verify("trigger/current_trigger",
 						    dev_dir_name,
 						    trigger_name);
@@ -334,8 +343,9 @@ int main(int argc, char **argv)
 	ret = write_sysfs_int("enable", buf_dir_name, 1);
 	if (ret < 0)
 		goto error_free_buf_dir_name;
+
 	scan_size = size_from_channelarray(channels, num_channels);
-	data = malloc(scan_size*buf_len);
+	data = malloc(scan_size * buf_len);
 	if (!data) {
 		ret = -ENOMEM;
 		goto error_free_buf_dir_name;
@@ -349,13 +359,12 @@ int main(int argc, char **argv)
 
 	/* Attempt to open non blocking the access dev */
 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
-	if (fp == -1) { /* If it isn't there make the node */
+	if (fp == -1) { /* TODO: If it isn't there make the node */
 		ret = -errno;
 		printf("Failed to open %s\n", buffer_access);
 		goto error_free_buffer_access;
 	}
 
-	/* Wait for events 10 times */
 	for (j = 0; j < num_loops; j++) {
 		if (!noevents) {
 			struct pollfd pfd = {
@@ -372,25 +381,22 @@ int main(int argc, char **argv)
 			}
 
 			toread = buf_len;
-
 		} else {
 			usleep(timedelay);
 			toread = 64;
 		}
 
-		read_size = read(fp,
-				 data,
-				 toread*scan_size);
+		read_size = read(fp, data, toread * scan_size);
 		if (read_size < 0) {
 			if (errno == EAGAIN) {
 				printf("nothing available\n");
 				continue;
-			} else
+			} else {
 				break;
+			}
 		}
-		for (i = 0; i < read_size/scan_size; i++)
-			process_scan(data + scan_size*i,
-				     channels,
+		for (i = 0; i < read_size / scan_size; i++)
+			process_scan(data + scan_size * i, channels,
 				     num_channels);
 	}
 
@@ -409,6 +415,7 @@ int main(int argc, char **argv)
 error_close_buffer_access:
 	if (close(fp) == -1)
 		perror("Failed to close buffer");
+
 error_free_buffer_access:
 	free(buffer_access);
 error_free_data:
@@ -424,6 +431,7 @@ error_free_channels:
 error_free_triggername:
 	if (datardytrigger && !notrigger)
 		free(trigger_name);
+
 error_free_dev_dir_name:
 	free(dev_dir_name);
 
tools/iio/iio_event_monitor.c +8 -5
index 8e682a7..c36f314 100644
@@ -13,7 +13,6 @@
  *
  * Usage:
  *	iio_event_monitor <device_name>
- *
  */
 
 #include <unistd.h>
@@ -209,7 +208,8 @@ static void print_event(struct iio_event_data *event)
 
 	if (!event_is_known(event)) {
 		printf("Unknown event: time: %lld, id: %llx\n",
-				event->timestamp, event->id);
+		       event->timestamp, event->id);
+
 		return;
 	}
 
@@ -229,6 +229,7 @@ static void print_event(struct iio_event_data *event)
 
 	if (dir != IIO_EV_DIR_NONE)
 		printf(", direction: %s", iio_ev_dir_text[dir]);
+
 	printf("\n");
 }
 
@@ -251,14 +252,16 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num >= 0) {
 		printf("Found IIO device with name %s with device number %d\n",
-			device_name, dev_num);
+		       device_name, dev_num);
 		ret = asprintf(&chrdev_name, "/dev/iio:device%d", dev_num);
 		if (ret < 0) {
 			return -ENOMEM;
 		}
 	} else {
-		/* If we can't find a IIO device by name assume device_name is a
-		   IIO chrdev */
+		/*
+		 * If we can't find an IIO device by name assume device_name is
+		 * an IIO chrdev
+		 */
 		chrdev_name = strdup(device_name);
 		if (!chrdev_name)
 			return -ENOMEM;
tools/iio/iio_utils.c +67 -44

438 lines not on this page · raw message

tools/iio/iio_utils.h +7 -8
index 379eed9..0866101 100644
@@ -51,17 +51,16 @@ struct iio_channel_info {
 };
 
 int iioutils_break_up_name(const char *full_name, char **generic_name);
-int iioutils_get_type(unsigned *is_signed, unsigned *bytes,
-					  unsigned *bits_used, unsigned *shift,
-					  uint64_t *mask, unsigned *be,
-					  const char *device_dir, const char *name,
-					  const char *generic_name);
+int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
+		      unsigned *shift, uint64_t *mask, unsigned *be,
+		      const char *device_dir, const char *name,
+		      const char *generic_name);
 int iioutils_get_param_float(float *output, const char *param_name,
-							 const char *device_dir, const char *name,
-							 const char *generic_name);
+			     const char *device_dir, const char *name,
+			     const char *generic_name);
 void bsort_channel_array_by_index(struct iio_channel_info **ci_array, int cnt);
 int build_channel_array(const char *device_dir,
-						struct iio_channel_info **ci_array, int *counter);
+			struct iio_channel_info **ci_array, int *counter);
 int find_type_by_name(const char *name, const char *type);
 int write_sysfs_int(const char *filename, const char *basedir, int val);
 int write_sysfs_int_and_verify(const char *filename, const char *basedir,
tools/iio/lsiio.c +9 -6
index b59ee17..f5b3bd0 100644
@@ -20,7 +20,6 @@
 #include <sys/dir.h>
 #include "iio_utils.h"
 
-
 static enum verbosity {
 	VERBLEVEL_DEFAULT,	/* 0 gives lspci behaviour */
 	VERBLEVEL_SENSORS,	/* 1 lists sensors */
@@ -29,17 +28,16 @@ static enum verbosity {
 const char *type_device = "iio:device";
 const char *type_trigger = "trigger";
 
-
 static inline int check_prefix(const char *str, const char *prefix)
 {
 	return strlen(str) > strlen(prefix) &&
-		strncmp(str, prefix, strlen(prefix)) == 0;
+	       strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
 static inline int check_postfix(const char *str, const char *postfix)
 {
 	return strlen(str) > strlen(postfix) &&
-		strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
+	       strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
 }
 
 static int dump_channels(const char *dev_dir_name)
@@ -50,11 +48,11 @@ static int dump_channels(const char *dev_dir_name)
 	dp = opendir(dev_dir_name);
 	if (dp == NULL)
 		return -errno;
+
 	while (ent = readdir(dp), ent != NULL)
 		if (check_prefix(ent->d_name, "in_") &&
-		    check_postfix(ent->d_name, "_raw")) {
+		    check_postfix(ent->d_name, "_raw"))
 			printf("   %-10s\n", ent->d_name);
-		}
 
 	return (closedir(dp) == -1) ? -errno : 0;
 }
@@ -69,6 +67,7 @@ static int dump_one_device(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
+
 	retval = read_sysfs_string("name", dev_dir_name, name);
 	if (retval)
 		return retval;
@@ -77,6 +76,7 @@ static int dump_one_device(const char *dev_dir_name)
 
 	if (verblevel >= VERBLEVEL_SENSORS)
 		return dump_channels(dev_dir_name);
+
 	return 0;
 }
 
@@ -90,11 +90,13 @@ static int dump_one_trigger(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
+
 	retval = read_sysfs_string("name", dev_dir_name, name);
 	if (retval)
 		return retval;
 
 	printf("Trigger %03d: %s\n", dev_idx, name);
+
 	return 0;
 }
 
@@ -151,6 +153,7 @@ static int dump_devices(void)
 			free(dev_dir_name);
 		}
 	}
+
 	return (closedir(dp) == -1) ? -errno : 0;
 
 error_close_dir:
Signature · 1 line
2.3.6
maintainer
#59 lore raw

Re: [PATCH 31/32] tools:iio: adjust coding style

Quoted 4 lines · Hartmut Knaack

Fix various coding style issues, including:
* have spaces around operators
* indentation
* consolidate parameters in same line

In the cases where we have to be multiline anyway I'm not sure I'm that keen on bringing them onto a minimal set of lines. It's slightly easier
to read if we don't. Still don't really care either way ;)

Quoted 8 lines

* required braces
* adjust/drop comments
* multiline comment style
* delete unnecessary empty lines
* add empty lines to visualize logial code blocks
* typos

This one won't apply yet as I'm holding the earlier patch back but will pick it up once any discussion on that is done.

Jonathan

Quoted 817 lines
---
 tools/iio/generic_buffer.c    |  46 +++++++++--------
 tools/iio/iio_event_monitor.c |  13 +++--
 tools/iio/iio_utils.c         | 111 +++++++++++++++++++++++++-----------------
 tools/iio/iio_utils.h         |  15 +++---
 tools/iio/lsiio.c             |  15 +++---
 5 files changed, 118 insertions(+), 82 deletions(-)
tools/iio/generic_buffer.c +27 -19
index c546852..a4c14d0 100644
@@ -51,11 +51,13 @@ int size_from_channelarray(struct iio_channel_info *channels, int num_channels)
 		if (bytes % channels[i].bytes == 0)
 			channels[i].location = bytes;
 		else
-			channels[i].location = bytes - bytes%channels[i].bytes
-				+ channels[i].bytes;
+			channels[i].location = bytes - bytes % channels[i].bytes
+					       + channels[i].bytes;
+
 		bytes = channels[i].location + channels[i].bytes;
 		i++;
 	}
+
 	return bytes;
 }
 
@@ -136,9 +138,9 @@ void print8byte(uint64_t input, struct iio_channel_info *info)
 /**
  * process_scan() - print out the values in SI units
  * @data:		pointer to the start of the scan
- * @channels:		information about the channels. Note
- *  size_from_channelarray must have been called first to fill the
- *  location offsets.
+ * @channels:		information about the channels.
+ *			Note: size_from_channelarray must have been called first
+ *			      to fill the location offsets.
  * @num_channels:	number of channels
  **/
 void process_scan(char *data,
@@ -213,6 +215,7 @@ int main(int argc, char **argv)
 			num_loops = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
+
 			break;
 		case 'e':
 			noevents = 1;
@@ -225,6 +228,7 @@ int main(int argc, char **argv)
 			buf_len = strtoul(optarg, &dummy, 10);
 			if (errno)
 				return -errno;
+
 			break;
 		case 'n':
 			device_name = optarg;
@@ -257,6 +261,7 @@ int main(int argc, char **argv)
 		printf("Failed to find the %s\n", device_name);
 		return dev_num;
 	}
+
 	printf("iio device number being used is %d\n", dev_num);
 
 	ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
@@ -285,9 +290,11 @@ int main(int argc, char **argv)
 			ret = trig_num;
 			goto error_free_triggername;
 		}
+
 		printf("iio trigger number being used is %d\n", trig_num);
-	} else
+	} else {
 		printf("trigger-less mode selected\n");
+	}
 
 	/*
 	 * Parse the files in scan_elements to identify what channels are
@@ -314,8 +321,10 @@ int main(int argc, char **argv)
 
 	if (!notrigger) {
 		printf("%s %s\n", dev_dir_name, trigger_name);
-		/* Set the device trigger to be the data ready trigger found
-		 * above */
+		/*
+		 * Set the device trigger to be the data ready trigger found
+		 * above
+		 */
 		ret = write_sysfs_string_and_verify("trigger/current_trigger",
 						    dev_dir_name,
 						    trigger_name);
@@ -334,8 +343,9 @@ int main(int argc, char **argv)
 	ret = write_sysfs_int("enable", buf_dir_name, 1);
 	if (ret < 0)
 		goto error_free_buf_dir_name;
+
 	scan_size = size_from_channelarray(channels, num_channels);
-	data = malloc(scan_size*buf_len);
+	data = malloc(scan_size * buf_len);
 	if (!data) {
 		ret = -ENOMEM;
 		goto error_free_buf_dir_name;
@@ -349,13 +359,12 @@ int main(int argc, char **argv)
 
 	/* Attempt to open non blocking the access dev */
 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
-	if (fp == -1) { /* If it isn't there make the node */
+	if (fp == -1) { /* TODO: If it isn't there make the node */
 		ret = -errno;
 		printf("Failed to open %s\n", buffer_access);
 		goto error_free_buffer_access;
 	}
 
-	/* Wait for events 10 times */
 	for (j = 0; j < num_loops; j++) {
 		if (!noevents) {
 			struct pollfd pfd = {
@@ -372,25 +381,22 @@ int main(int argc, char **argv)
 			}
 
 			toread = buf_len;
-
 		} else {
 			usleep(timedelay);
 			toread = 64;
 		}
 
-		read_size = read(fp,
-				 data,
-				 toread*scan_size);
+		read_size = read(fp, data, toread * scan_size);
 		if (read_size < 0) {
 			if (errno == EAGAIN) {
 				printf("nothing available\n");
 				continue;
-			} else
+			} else {
 				break;
+			}
 		}
-		for (i = 0; i < read_size/scan_size; i++)
-			process_scan(data + scan_size*i,
-				     channels,
+		for (i = 0; i < read_size / scan_size; i++)
+			process_scan(data + scan_size * i, channels,
 				     num_channels);
 	}
 
@@ -409,6 +415,7 @@ int main(int argc, char **argv)
 error_close_buffer_access:
 	if (close(fp) == -1)
 		perror("Failed to close buffer");
+
 error_free_buffer_access:
 	free(buffer_access);
 error_free_data:
@@ -424,6 +431,7 @@ error_free_channels:
 error_free_triggername:
 	if (datardytrigger && !notrigger)
 		free(trigger_name);
+
 error_free_dev_dir_name:
 	free(dev_dir_name);
 
tools/iio/iio_event_monitor.c +8 -5
index 8e682a7..c36f314 100644
@@ -13,7 +13,6 @@
  *
  * Usage:
  *	iio_event_monitor <device_name>
- *
  */
 
 #include <unistd.h>
@@ -209,7 +208,8 @@ static void print_event(struct iio_event_data *event)
 
 	if (!event_is_known(event)) {
 		printf("Unknown event: time: %lld, id: %llx\n",
-				event->timestamp, event->id);
+		       event->timestamp, event->id);
+
 		return;
 	}
 
@@ -229,6 +229,7 @@ static void print_event(struct iio_event_data *event)
 
 	if (dir != IIO_EV_DIR_NONE)
 		printf(", direction: %s", iio_ev_dir_text[dir]);
+
 	printf("\n");
 }
 
@@ -251,14 +252,16 @@ int main(int argc, char **argv)
 	dev_num = find_type_by_name(device_name, "iio:device");
 	if (dev_num >= 0) {
 		printf("Found IIO device with name %s with device number %d\n",
-			device_name, dev_num);
+		       device_name, dev_num);
 		ret = asprintf(&chrdev_name, "/dev/iio:device%d", dev_num);
 		if (ret < 0) {
 			return -ENOMEM;
 		}
 	} else {
-		/* If we can't find a IIO device by name assume device_name is a
-		   IIO chrdev */
+		/*
+		 * If we can't find an IIO device by name assume device_name is
+		 * an IIO chrdev
+		 */
 		chrdev_name = strdup(device_name);
 		if (!chrdev_name)
 			return -ENOMEM;
tools/iio/iio_utils.c +67 -44
index ec9ab7f..4eac11a 100644
@@ -32,8 +32,7 @@ static char * const iio_direction[] = {
  *
  * Returns 0 on success, or a negative error code if string extraction failed.
  **/
-int iioutils_break_up_name(const char *full_name,
-				  char **generic_name)
+int iioutils_break_up_name(const char *full_name, char **generic_name)
 {
 	char *current;
 	char *w, *r;
@@ -65,6 +64,7 @@ int iioutils_break_up_name(const char *full_name,
 			*w = *r;
 			w++;
 		}
+
 		r++;
 	}
 	*w = '\0';
@@ -88,15 +88,10 @@ int iioutils_break_up_name(const char *full_name,
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int iioutils_get_type(unsigned *is_signed,
-			     unsigned *bytes,
-			     unsigned *bits_used,
-			     unsigned *shift,
-			     uint64_t *mask,
-			     unsigned *be,
-			     const char *device_dir,
-			     const char *name,
-			     const char *generic_name)
+int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
+		      unsigned *shift, uint64_t *mask, unsigned *be,
+		      const char *device_dir, const char *name,
+		      const char *generic_name)
 {
 	FILE *sysfsfp;
 	int ret;
@@ -126,6 +121,7 @@ int iioutils_get_type(unsigned *is_signed,
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+
 	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		/*
@@ -140,6 +136,7 @@ int iioutils_get_type(unsigned *is_signed,
 				ret = -ENOMEM;
 				goto error_closedir;
 			}
+
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
 				ret = -errno;
@@ -162,12 +159,14 @@ int iioutils_get_type(unsigned *is_signed,
 				printf("scan type description didn't match\n");
 				goto error_close_sysfsfp;
 			}
+
 			*be = (endianchar == 'b');
 			*bytes = padint / 8;
 			if (*bits_used == 64)
 				*mask = ~0;
 			else
 				*mask = (1 << *bits_used) - 1;
+
 			*is_signed = (signchar == 's');
 			if (fclose(sysfsfp)) {
 				ret = -errno;
@@ -177,9 +176,9 @@ int iioutils_get_type(unsigned *is_signed,
 
 			sysfsfp = 0;
 			free(filename);
-
 			filename = 0;
 		}
+
 error_close_sysfsfp:
 	if (sysfsfp)
 		if (fclose(sysfsfp))
@@ -188,6 +187,7 @@ error_close_sysfsfp:
 error_free_filename:
 	if (filename)
 		free(filename);
+
 error_closedir:
 	if (closedir(dp) == -1)
 		perror("iioutils_get_type(): Failed to close directory");
@@ -212,11 +212,9 @@ error_free_scan_el_dir:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int iioutils_get_param_float(float *output,
-				    const char *param_name,
-				    const char *device_dir,
-				    const char *name,
-				    const char *generic_name)
+int iioutils_get_param_float(float *output, const char *param_name,
+			     const char *device_dir, const char *name,
+			     const char *generic_name)
 {
 	FILE *sysfsfp;
 	int ret;
@@ -235,11 +233,13 @@ int iioutils_get_param_float(float *output,
 		ret = -ENOMEM;
 		goto error_free_builtname;
 	}
+
 	dp = opendir(device_dir);
 	if (dp == NULL) {
 		ret = -errno;
 		goto error_free_builtname_generic;
 	}
+
 	ret = -ENOENT;
 	while (ent = readdir(dp), ent != NULL)
 		if ((strcmp(builtname, ent->d_name) == 0) ||
@@ -250,11 +250,13 @@ int iioutils_get_param_float(float *output,
 				ret = -ENOMEM;
 				goto error_closedir;
 			}
+
 			sysfsfp = fopen(filename, "r");
 			if (!sysfsfp) {
 				ret = -errno;
 				goto error_free_filename;
 			}
+
 			errno = 0;
 			if (fscanf(sysfsfp, "%f", output) != 1)
 				ret = errno ? -errno : -ENODATA;
@@ -264,6 +266,7 @@ int iioutils_get_param_float(float *output,
 error_free_filename:
 	if (filename)
 		free(filename);
+
 error_closedir:
 	if (closedir(dp) == -1)
 		perror("iioutils_get_param_float(): Failed to close directory");
@@ -282,16 +285,14 @@ error_free_builtname:
  * @cnt: the amount of array elements
  **/
 
-void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
-					 int cnt)
+void bsort_channel_array_by_index(struct iio_channel_info **ci_array, int cnt)
 {
-
 	struct iio_channel_info temp;
 	int x, y;
 
 	for (x = 0; x < cnt; x++)
 		for (y = 0; y < (cnt - 1); y++)
-			if ((*ci_array)[y].index > (*ci_array)[y+1].index) {
+			if ((*ci_array)[y].index > (*ci_array)[y + 1].index) {
 				temp = (*ci_array)[y + 1];
 				(*ci_array)[y + 1] = (*ci_array)[y];
 				(*ci_array)[y] = temp;
@@ -307,8 +308,7 @@ void bsort_channel_array_by_index(struct iio_channel_info **ci_array,
  * Returns 0 on success, otherwise a negative error code.
  **/
 int build_channel_array(const char *device_dir,
-			      struct iio_channel_info **ci_array,
-			      int *counter)
+			struct iio_channel_info **ci_array, int *counter)
 {
 	DIR *dp;
 	FILE *sysfsfp;
@@ -329,6 +329,7 @@ int build_channel_array(const char *device_dir,
 		ret = -errno;
 		goto error_free_name;
 	}
+
 	while (ent = readdir(dp), ent != NULL)
 		if (strcmp(ent->d_name + strlen(ent->d_name) - strlen("_en"),
 			   "_en") == 0) {
@@ -338,12 +339,14 @@ int build_channel_array(const char *device_dir,
 				ret = -ENOMEM;
 				goto error_close_dir;
 			}
+
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
 				ret = -errno;
 				free(filename);
 				goto error_close_dir;
 			}
+
 			errno = 0;
 			if (fscanf(sysfsfp, "%i", &ret) != 1) {
 				ret = errno ? -errno : -ENODATA;
@@ -353,9 +356,9 @@ int build_channel_array(const char *device_dir,
 				free(filename);
 				goto error_close_dir;
 			}
-
 			if (ret == 1)
 				(*counter)++;
+
 			if (fclose(sysfsfp)) {
 				ret = -errno;
 				free(filename);
@@ -364,11 +367,13 @@ int build_channel_array(const char *device_dir,
 
 			free(filename);
 		}
+
 	*ci_array = malloc(sizeof(**ci_array) * (*counter));
 	if (*ci_array == NULL) {
 		ret = -ENOMEM;
 		goto error_close_dir;
 	}
+
 	seekdir(dp, 0);
 	while (ent = readdir(dp), ent != NULL) {
 		if (strcmp(ent->d_name + strlen(ent->d_name) - strlen("_en"),
@@ -384,6 +389,7 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
+
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
 				ret = -errno;
@@ -391,6 +397,7 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
+
 			errno = 0;
 			if (fscanf(sysfsfp, "%i", &current_enabled) != 1) {
 				ret = errno ? -errno : -ENODATA;
@@ -423,6 +430,7 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
+
 			/* Get the generic and specific name elements */
 			ret = iioutils_break_up_name(current->name,
 						     &current->generic_name);
@@ -432,6 +440,7 @@ int build_channel_array(const char *device_dir,
 				count--;
 				goto error_cleanup_array;
 			}
+
 			ret = asprintf(&filename,
 				       "%s/%s_index",
 				       scan_el_dir,
@@ -441,6 +450,7 @@ int build_channel_array(const char *device_dir,
 				ret = -ENOMEM;
 				goto error_cleanup_array;
 			}
+
 			sysfsfp = fopen(filename, "r");
 			if (sysfsfp == NULL) {
 				ret = -errno;
@@ -474,6 +484,7 @@ int build_channel_array(const char *device_dir,
 						       current->generic_name);
 			if (ret < 0)
 				goto error_cleanup_array;
+
 			ret = iioutils_get_param_float(&current->offset,
 						       "offset",
 						       device_dir,
@@ -481,6 +492,7 @@ int build_channel_array(const char *device_dir,
 						       current->generic_name);
 			if (ret < 0)
 				goto error_cleanup_array;
+
 			ret = iioutils_get_type(&current->is_signed,
 						&current->bytes,
 						&current->bits_used,
@@ -562,9 +574,9 @@ int find_type_by_name(const char *name, const char *type)
 
 	while (ent = readdir(dp), ent != NULL) {
 		if (strcmp(ent->d_name, ".") != 0 &&
-			strcmp(ent->d_name, "..") != 0 &&
-			strlen(ent->d_name) > strlen(type) &&
-			strncmp(ent->d_name, type, strlen(type)) == 0) {
+		    strcmp(ent->d_name, "..") != 0 &&
+		    strlen(ent->d_name) > strlen(type) &&
+		    strncmp(ent->d_name, type, strlen(type)) == 0) {
 			errno = 0;
 			ret = sscanf(ent->d_name + strlen(type), "%d", &number);
 			if (ret < 0) {
@@ -580,12 +592,9 @@ int find_type_by_name(const char *name, const char *type)
 			numstrlen = calc_digits(number);
 			/* verify the next character is not a colon */
 			if (strncmp(ent->d_name + strlen(type) + numstrlen,
-					":",
-					1) != 0) {
-				filename = malloc(strlen(iio_dir)
-						+ strlen(type)
-						+ numstrlen
-						+ 6);
+			    ":", 1) != 0) {
+				filename = malloc(strlen(iio_dir) + strlen(type)
+						  + numstrlen + 6);
 				if (filename == NULL) {
 					ret = -ENOMEM;
 					goto error_close_dir;
@@ -603,6 +612,7 @@ int find_type_by_name(const char *name, const char *type)
 					free(filename);
 					continue;
 				}
+
 				free(filename);
 				errno = 0;
 				if (fscanf(nameFile, "%s", thisname) != 1) {
@@ -618,6 +628,7 @@ int find_type_by_name(const char *name, const char *type)
 				if (strcmp(name, thisname) == 0) {
 					if (closedir(dp) == -1)
 						return -errno;
+
 					return number;
 				}
 			}
@@ -631,6 +642,7 @@ int find_type_by_name(const char *name, const char *type)
 error_close_dir:
 	if (closedir(dp) == -1)
 		perror("find_type_by_name(): Failed to close directory");
+
 	return ret;
 }
 
@@ -644,6 +656,7 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
 
 	if (temp == NULL)
 		return -ENOMEM;
+
 	ret = sprintf(temp, "%s/%s", basedir, filename);
 	if (ret < 0)
 		goto error_free;
@@ -654,6 +667,7 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
 		printf("failed to open %s\n", temp);
 		goto error_free;
 	}
+
 	ret = fprintf(sysfsfp, "%d", val);
 	if (ret < 0) {
 		if (fclose(sysfsfp))
@@ -674,6 +688,7 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
 			printf("failed to open %s\n", temp);
 			goto error_free;
 		}
+
 		if (fscanf(sysfsfp, "%d", &test) != 1) {
 			ret = errno ? -errno : -ENODATA;
 			if (fclose(sysfsfp))
@@ -688,13 +703,12 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
 		}
 
 		if (test != val) {
-			printf("Possible failure in int write %d to %s%s\n",
-				val,
-				basedir,
-				filename);
+			printf("Possible failure in int write %d to %s/%s\n",
+			       val, basedir, filename);
 			ret = -1;
 		}
 	}
+
 error_free:
 	free(temp);
 	return ret;
@@ -739,6 +753,7 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
 		printf("Memory allocation failed\n");
 		return -ENOMEM;
 	}
+
 	ret = sprintf(temp, "%s/%s", basedir, filename);
 	if (ret < 0)
 		goto error_free;
@@ -749,6 +764,7 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
 		printf("Could not open %s\n", temp);
 		goto error_free;
 	}
+
 	ret = fprintf(sysfsfp, "%s", val);
 	if (ret < 0) {
 		if (fclose(sysfsfp))
@@ -766,9 +782,10 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
 		sysfsfp = fopen(temp, "r");
 		if (sysfsfp == NULL) {
 			ret = -errno;
-			printf("could not open file to verify\n");
+			printf("Could not open file to verify\n");
 			goto error_free;
 		}
+
 		if (fscanf(sysfsfp, "%s", temp) != 1) {
 			ret = errno ? -errno : -ENODATA;
 			if (fclose(sysfsfp))
@@ -784,15 +801,12 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
 
 		if (strcmp(temp, val) != 0) {
 			printf("Possible failure in string write of %s "
-				"Should be %s "
-				"written to %s\%s\n",
-				temp,
-				val,
-				basedir,
-				filename);
+			       "Should be %s written to %s/%s\n", temp, val,
+			       basedir, filename);
 			ret = -1;
 		}
 	}
+
 error_free:
 	free(temp);
 
@@ -845,6 +859,7 @@ int read_sysfs_posint(const char *filename, const char *basedir)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
+
 	ret = sprintf(temp, "%s/%s", basedir, filename);
 	if (ret < 0)
 		goto error_free;
@@ -854,6 +869,7 @@ int read_sysfs_posint(const char *filename, const char *basedir)
 		ret = -errno;
 		goto error_free;
 	}
+
 	errno = 0;
 	if (fscanf(sysfsfp, "%d\n", &ret) != 1) {
 		ret = errno ? -errno : -ENODATA;
@@ -868,6 +884,7 @@ int read_sysfs_posint(const char *filename, const char *basedir)
 
 error_free:
 	free(temp);
+
 	return ret;
 }
 
@@ -889,6 +906,7 @@ int read_sysfs_float(const char *filename, const char *basedir, float *val)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
+
 	ret = sprintf(temp, "%s/%s", basedir, filename);
 	if (ret < 0)
 		goto error_free;
@@ -898,6 +916,7 @@ int read_sysfs_float(const char *filename, const char *basedir, float *val)
 		ret = -errno;
 		goto error_free;
 	}
+
 	errno = 0;
 	if (fscanf(sysfsfp, "%f\n", val) != 1) {
 		ret = errno ? -errno : -ENODATA;
@@ -912,6 +931,7 @@ int read_sysfs_float(const char *filename, const char *basedir, float *val)
 
 error_free:
 	free(temp);
+
 	return ret;
 }
 
@@ -933,6 +953,7 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
 		printf("Memory allocation failed");
 		return -ENOMEM;
 	}
+
 	ret = sprintf(temp, "%s/%s", basedir, filename);
 	if (ret < 0)
 		goto error_free;
@@ -942,6 +963,7 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
 		ret = -errno;
 		goto error_free;
 	}
+
 	errno = 0;
 	if (fscanf(sysfsfp, "%s\n", str) != 1) {
 		ret = errno ? -errno : -ENODATA;
@@ -956,6 +978,7 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
 
 error_free:
 	free(temp);
+
 	return ret;
 }
 
tools/iio/iio_utils.h +7 -8
index 379eed9..0866101 100644
@@ -51,17 +51,16 @@ struct iio_channel_info {
 };
 
 int iioutils_break_up_name(const char *full_name, char **generic_name);
-int iioutils_get_type(unsigned *is_signed, unsigned *bytes,
-					  unsigned *bits_used, unsigned *shift,
-					  uint64_t *mask, unsigned *be,
-					  const char *device_dir, const char *name,
-					  const char *generic_name);
+int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
+		      unsigned *shift, uint64_t *mask, unsigned *be,
+		      const char *device_dir, const char *name,
+		      const char *generic_name);
 int iioutils_get_param_float(float *output, const char *param_name,
-							 const char *device_dir, const char *name,
-							 const char *generic_name);
+			     const char *device_dir, const char *name,
+			     const char *generic_name);
 void bsort_channel_array_by_index(struct iio_channel_info **ci_array, int cnt);
 int build_channel_array(const char *device_dir,
-						struct iio_channel_info **ci_array, int *counter);
+			struct iio_channel_info **ci_array, int *counter);
 int find_type_by_name(const char *name, const char *type);
 int write_sysfs_int(const char *filename, const char *basedir, int val);
 int write_sysfs_int_and_verify(const char *filename, const char *basedir,
tools/iio/lsiio.c +9 -6
index b59ee17..f5b3bd0 100644
@@ -20,7 +20,6 @@
 #include <sys/dir.h>
 #include "iio_utils.h"
 
-
 static enum verbosity {
 	VERBLEVEL_DEFAULT,	/* 0 gives lspci behaviour */
 	VERBLEVEL_SENSORS,	/* 1 lists sensors */
@@ -29,17 +28,16 @@ static enum verbosity {
 const char *type_device = "iio:device";
 const char *type_trigger = "trigger";
 
-
 static inline int check_prefix(const char *str, const char *prefix)
 {
 	return strlen(str) > strlen(prefix) &&
-		strncmp(str, prefix, strlen(prefix)) == 0;
+	       strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
 static inline int check_postfix(const char *str, const char *postfix)
 {
 	return strlen(str) > strlen(postfix) &&
-		strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
+	       strcmp(str + strlen(str) - strlen(postfix), postfix) == 0;
 }
 
 static int dump_channels(const char *dev_dir_name)
@@ -50,11 +48,11 @@ static int dump_channels(const char *dev_dir_name)
 	dp = opendir(dev_dir_name);
 	if (dp == NULL)
 		return -errno;
+
 	while (ent = readdir(dp), ent != NULL)
 		if (check_prefix(ent->d_name, "in_") &&
-		    check_postfix(ent->d_name, "_raw")) {
+		    check_postfix(ent->d_name, "_raw"))
 			printf("   %-10s\n", ent->d_name);
-		}
 
 	return (closedir(dp) == -1) ? -errno : 0;
 }
@@ -69,6 +67,7 @@ static int dump_one_device(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
+
 	retval = read_sysfs_string("name", dev_dir_name, name);
 	if (retval)
 		return retval;
@@ -77,6 +76,7 @@ static int dump_one_device(const char *dev_dir_name)
 
 	if (verblevel >= VERBLEVEL_SENSORS)
 		return dump_channels(dev_dir_name);
+
 	return 0;
 }
 
@@ -90,11 +90,13 @@ static int dump_one_trigger(const char *dev_dir_name)
 			"%i", &dev_idx);
 	if (retval != 1)
 		return -EINVAL;
+
 	retval = read_sysfs_string("name", dev_dir_name, name);
 	if (retval)
 		return retval;
 
 	printf("Trigger %03d: %s\n", dev_idx, name);
+
 	return 0;
 }
 
@@ -151,6 +153,7 @@ static int dump_devices(void)
 			free(dev_dir_name);
 		}
 	}
+
 	return (closedir(dp) == -1) ? -errno : 0;
 
 error_close_dir:
#60 lore raw

PATCH 32/32 tools:iio: rename variables

In reply to Hartmut Knaack

Use more appropriate/common variable names:
* namepf instead of nameFile in iio_utils.c
* ret instead of retval in lsiio.c

---
 tools/iio/iio_utils.c | 10 +++++-----
 tools/iio/lsiio.c     | 28 ++++++++++++++--------------
 2 files changed, 19 insertions(+), 19 deletions(-)
tools/iio/iio_utils.c +5 -5
index 4eac11a..8fb3214 100644
@@ -561,7 +561,7 @@ int find_type_by_name(const char *name, const char *type)
 	const struct dirent *ent;
 	int number, numstrlen, ret;
 
-	FILE *nameFile;
+	FILE *namefp;
 	DIR *dp;
 	char thisname[IIO_MAX_NAME_LENGTH];
 	char *filename;
@@ -607,20 +607,20 @@ int find_type_by_name(const char *name, const char *type)
 					goto error_close_dir;
 				}
 
-				nameFile = fopen(filename, "r");
-				if (!nameFile) {
+				namefp = fopen(filename, "r");
+				if (!namefp) {
 					free(filename);
 					continue;
 				}
 
 				free(filename);
 				errno = 0;
-				if (fscanf(nameFile, "%s", thisname) != 1) {
+				if (fscanf(namefp, "%s", thisname) != 1) {
 					ret = errno ? -errno : -ENODATA;
 					goto error_close_dir;
 				}
 
-				if (fclose(nameFile)) {
+				if (fclose(namefp)) {
 					ret = -errno;
 					goto error_close_dir;
 				}
tools/iio/lsiio.c +14 -14
index f5b3bd0..7f432a5 100644
@@ -61,16 +61,16 @@ static int dump_one_device(const char *dev_dir_name)
 {
 	char name[IIO_MAX_NAME_LENGTH];
 	int dev_idx;
-	int retval;
+	int ret;
 
-	retval = sscanf(dev_dir_name + strlen(iio_dir) + strlen(type_device),
-			"%i", &dev_idx);
-	if (retval != 1)
+	ret = sscanf(dev_dir_name + strlen(iio_dir) + strlen(type_device), "%i",
+		     &dev_idx);
+	if (ret != 1)
 		return -EINVAL;
 
-	retval = read_sysfs_string("name", dev_dir_name, name);
-	if (retval)
-		return retval;
+	ret = read_sysfs_string("name", dev_dir_name, name);
+	if (ret)
+		return ret;
 
 	printf("Device %03d: %s\n", dev_idx, name);
 
@@ -84,16 +84,16 @@ static int dump_one_trigger(const char *dev_dir_name)
 {
 	char name[IIO_MAX_NAME_LENGTH];
 	int dev_idx;
-	int retval;
+	int ret;
 
-	retval = sscanf(dev_dir_name + strlen(iio_dir) + strlen(type_trigger),
-			"%i", &dev_idx);
-	if (retval != 1)
+	ret = sscanf(dev_dir_name + strlen(iio_dir) + strlen(type_trigger),
+		     "%i", &dev_idx);
+	if (ret != 1)
 		return -EINVAL;
 
-	retval = read_sysfs_string("name", dev_dir_name, name);
-	if (retval)
-		return retval;
+	ret = read_sysfs_string("name", dev_dir_name, name);
+	if (ret)
+		return ret;
 
 	printf("Trigger %03d: %s\n", dev_idx, name);
 
Signature · 1 line
2.3.6
maintainer Applied
#61 lore raw

Re: [PATCH 30/32] tools:iio:iio_utils: pass strings as const

In reply to Hartmut Knaack
Quoted 4 lines · Hartmut Knaack

Mark strings, which are not supposed to be changed (basedir, filename, value), as const in function parameters.

Applied.

Quoted 111 lines
---
 tools/iio/iio_utils.c | 22 +++++++++++++---------
 tools/iio/iio_utils.h | 15 +++++++++------
 2 files changed, 22 insertions(+), 15 deletions(-)
tools/iio/iio_utils.c +13 -9
index dfee1a3..ec9ab7f 100644
@@ -634,7 +634,8 @@ error_close_dir:
 	return ret;
 }
 
-static int _write_sysfs_int(char *filename, char *basedir, int val, int verify)
+static int _write_sysfs_int(const char *filename, const char *basedir, int val,
+			    int verify)
 {
 	int ret = 0;
 	FILE *sysfsfp;
@@ -707,7 +708,7 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_int(char *filename, char *basedir, int val)
+int write_sysfs_int(const char *filename, const char *basedir, int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 0);
 }
@@ -721,13 +722,14 @@ int write_sysfs_int(char *filename, char *basedir, int val)
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_int_and_verify(char *filename, char *basedir, int val)
+int write_sysfs_int_and_verify(const char *filename, const char *basedir,
+			       int val)
 {
 	return _write_sysfs_int(filename, basedir, val, 1);
 }
 
-static int _write_sysfs_string(char *filename, char *basedir, char *val,
-			       int verify)
+static int _write_sysfs_string(const char *filename, const char *basedir,
+			       const char *val, int verify)
 {
 	int ret = 0;
 	FILE  *sysfsfp;
@@ -805,7 +807,8 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
+int write_sysfs_string_and_verify(const char *filename, const char *basedir,
+				  const char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 1);
 }
@@ -818,7 +821,8 @@ int write_sysfs_string_and_verify(char *filename, char *basedir, char *val)
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int write_sysfs_string(char *filename, char *basedir, char *val)
+int write_sysfs_string(const char *filename, const char *basedir,
+		       const char *val)
 {
 	return _write_sysfs_string(filename, basedir, val, 0);
 }
@@ -831,7 +835,7 @@ int write_sysfs_string(char *filename, char *basedir, char *val)
  * Returns the read integer value >= 0 on success, otherwise a negative error
  * code.
  **/
-int read_sysfs_posint(char *filename, char *basedir)
+int read_sysfs_posint(const char *filename, const char *basedir)
 {
 	int ret;
 	FILE  *sysfsfp;
@@ -875,7 +879,7 @@ error_free:
  *
  * Returns a value >= 0 on success, otherwise a negative error code.
  **/
-int read_sysfs_float(char *filename, char *basedir, float *val)
+int read_sysfs_float(const char *filename, const char *basedir, float *val)
 {
 	int ret = 0;
 	FILE  *sysfsfp;
tools/iio/iio_utils.h +9 -6
index e5eb46a..379eed9 100644
@@ -63,12 +63,15 @@ void bsort_channel_array_by_index(struct iio_channel_info **ci_array, int cnt);
 int build_channel_array(const char *device_dir,
 						struct iio_channel_info **ci_array, int *counter);
 int find_type_by_name(const char *name, const char *type);
-int write_sysfs_int(char *filename, char *basedir, int val);
-int write_sysfs_int_and_verify(char *filename, char *basedir, int val);
-int write_sysfs_string_and_verify(char *filename, char *basedir, char *val);
-int write_sysfs_string(char *filename, char *basedir, char *val);
-int read_sysfs_posint(char *filename, char *basedir);
-int read_sysfs_float(char *filename, char *basedir, float *val);
+int write_sysfs_int(const char *filename, const char *basedir, int val);
+int write_sysfs_int_and_verify(const char *filename, const char *basedir,
+			       int val);
+int write_sysfs_string_and_verify(const char *filename, const char *basedir,
+				  const char *val);
+int write_sysfs_string(const char *filename, const char *basedir,
+		       const char *val);
+int read_sysfs_posint(const char *filename, const char *basedir);
+int read_sysfs_float(const char *filename, const char *basedir, float *val);
 int read_sysfs_string(const char *filename, const char *basedir, char *str);
 
 #endif /* _IIO_UTILS_H_ */