lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Sun, 19 Jul 2015 15:04:45 +0100
From:	Jonathan Cameron <jic23@...nel.org>
To:	Cristina Opriceana <cristina.opriceana@...il.com>, knaack.h@....de
CC:	lars@...afoo.de, pmeerw@...erw.net, linux-iio@...r.kernel.org,
	linux-kernel@...r.kernel.org, daniel.baluta@...el.com,
	octavian.purdila@...el.com
Subject: Re: [PATCH v4] tools: iio: Send error messages to stderr

On 17/07/15 16:43, Cristina Opriceana wrote:
> This patch indends to make some cleanup and send printf
> error messages to stderr. The changes were performed with coccinelle
> for failure messages and manual for other cases, such as wrong usage
> messages.
> 
> Signed-off-by: Cristina Opriceana <cristina.opriceana@...il.com>
> Reviewed-by: Hartmut Knaack <knaack.h@....de>
Another good cleanup.  My only trivial quible.  You didn't mention
it was dependent on your previous set.  As I tend to work backwards through
my emails (half of them have been solved already by the time I get to them)
I was confused for a while!

Applied to the togreg branch of iio.git

Thanks,

Jonathan
> ---
> Changes since v4:
>  - fix indentation issues
> 
>  tools/iio/generic_buffer.c    | 39 ++++++++++++++++++---------------
>  tools/iio/iio_event_monitor.c | 14 ++++++------
>  tools/iio/iio_utils.c         | 51 +++++++++++++++++++++++++------------------
>  tools/iio/lsiio.c             |  2 +-
>  4 files changed, 59 insertions(+), 47 deletions(-)
> 
> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
> index 9535c2d..32f389eb 100644
> --- a/tools/iio/generic_buffer.c
> +++ b/tools/iio/generic_buffer.c
> @@ -193,15 +193,15 @@ void process_scan(char *data,
>  
>  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");
> +	fprintf(stderr, "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)
> @@ -271,7 +271,7 @@ int main(int argc, char **argv)
>  	}
>  
>  	if (!device_name) {
> -		printf("Device name not set\n");
> +		fprintf(stderr, "Device name not set\n");
>  		print_usage();
>  		return -1;
>  	}
> @@ -279,7 +279,7 @@ int main(int argc, char **argv)
>  	/* Find the device requested */
>  	dev_num = find_type_by_name(device_name, "iio:device");
>  	if (dev_num < 0) {
> -		printf("Failed to find the %s\n", device_name);
> +		fprintf(stderr, "Failed to find the %s\n", device_name);
>  		return dev_num;
>  	}
>  
> @@ -307,7 +307,8 @@ int main(int argc, char **argv)
>  		/* Verify the trigger exists */
>  		trig_num = find_type_by_name(trigger_name, "trigger");
>  		if (trig_num < 0) {
> -			printf("Failed to find the trigger %s\n", trigger_name);
> +			fprintf(stderr, "Failed to find the trigger %s\n",
> +				trigger_name);
>  			ret = trig_num;
>  			goto error_free_triggername;
>  		}
> @@ -323,8 +324,8 @@ int main(int argc, char **argv)
>  	 */
>  	ret = build_channel_array(dev_dir_name, &channels, &num_channels);
>  	if (ret) {
> -		printf("Problem reading scan element information\n");
> -		printf("diag %s\n", dev_dir_name);
> +		fprintf(stderr, "Problem reading scan element information\n"
> +			"diag %s\n", dev_dir_name);
>  		goto error_free_triggername;
>  	}
>  
> @@ -350,7 +351,8 @@ int main(int argc, char **argv)
>  						    dev_dir_name,
>  						    trigger_name);
>  		if (ret < 0) {
> -			printf("Failed to write current_trigger file\n");
> +			fprintf(stderr,
> +				"Failed to write current_trigger file\n");
>  			goto error_free_buf_dir_name;
>  		}
>  	}
> @@ -382,7 +384,7 @@ int main(int argc, char **argv)
>  	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
>  	if (fp == -1) { /* TODO: If it isn't there make the node */
>  		ret = -errno;
> -		printf("Failed to open %s\n", buffer_access);
> +		fprintf(stderr, "Failed to open %s\n", buffer_access);
>  		goto error_free_buffer_access;
>  	}
>  
> @@ -410,7 +412,7 @@ int main(int argc, char **argv)
>  		read_size = read(fp, data, toread * scan_size);
>  		if (read_size < 0) {
>  			if (errno == EAGAIN) {
> -				printf("nothing available\n");
> +				fprintf(stderr, "nothing available\n");
>  				continue;
>  			} else {
>  				break;
> @@ -431,7 +433,8 @@ int main(int argc, char **argv)
>  		ret = write_sysfs_string("trigger/current_trigger",
>  					 dev_dir_name, "NULL");
>  		if (ret < 0)
> -			printf("Failed to write to %s\n", dev_dir_name);
> +			fprintf(stderr, "Failed to write to %s\n",
> +				dev_dir_name);
>  
>  error_close_buffer_access:
>  	if (close(fp) == -1)
> diff --git a/tools/iio/iio_event_monitor.c b/tools/iio/iio_event_monitor.c
> index 9ee1959..cd3fd41 100644
> --- a/tools/iio/iio_event_monitor.c
> +++ b/tools/iio/iio_event_monitor.c
> @@ -215,8 +215,8 @@ static void print_event(struct iio_event_data *event)
>  	bool diff = IIO_EVENT_CODE_EXTRACT_DIFF(event->id);
>  
>  	if (!event_is_known(event)) {
> -		printf("Unknown event: time: %lld, id: %llx\n",
> -		       event->timestamp, event->id);
> +		fprintf(stderr, "Unknown event: time: %lld, id: %llx\n",
> +			event->timestamp, event->id);
>  
>  		return;
>  	}
> @@ -251,7 +251,7 @@ int main(int argc, char **argv)
>  	int fd, event_fd;
>  
>  	if (argc <= 1) {
> -		printf("Usage: %s <device_name>\n", argv[0]);
> +		fprintf(stderr, "Usage: %s <device_name>\n", argv[0]);
>  		return -1;
>  	}
>  
> @@ -277,14 +277,14 @@ int main(int argc, char **argv)
>  	fd = open(chrdev_name, 0);
>  	if (fd == -1) {
>  		ret = -errno;
> -		fprintf(stdout, "Failed to open %s\n", chrdev_name);
> +		fprintf(stderr, "Failed to open %s\n", chrdev_name);
>  		goto error_free_chrdev_name;
>  	}
>  
>  	ret = ioctl(fd, IIO_GET_EVENT_FD_IOCTL, &event_fd);
>  	if (ret == -1 || event_fd == -1) {
>  		ret = -errno;
> -		fprintf(stdout, "Failed to retrieve event fd\n");
> +		fprintf(stderr, "Failed to retrieve event fd\n");
>  		if (close(fd) == -1)
>  			perror("Failed to close character device file");
>  
> @@ -300,7 +300,7 @@ int main(int argc, char **argv)
>  		ret = read(event_fd, &event, sizeof(event));
>  		if (ret == -1) {
>  			if (errno == EAGAIN) {
> -				printf("nothing available\n");
> +				fprintf(stderr, "nothing available\n");
>  				continue;
>  			} else {
>  				ret = -errno;
> @@ -310,7 +310,7 @@ int main(int argc, char **argv)
>  		}
>  
>  		if (ret != sizeof(event)) {
> -			printf("Reading event failed!\n");
> +			fprintf(stderr, "Reading event failed!\n");
>  			ret = -EIO;
>  			break;
>  		}
> diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c
> index e177f40..c3f9e37 100644
> --- a/tools/iio/iio_utils.c
> +++ b/tools/iio/iio_utils.c
> @@ -140,7 +140,8 @@ int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
>  			sysfsfp = fopen(filename, "r");
>  			if (!sysfsfp) {
>  				ret = -errno;
> -				printf("failed to open %s\n", filename);
> +				fprintf(stderr, "failed to open %s\n",
> +					filename);
>  				goto error_free_filename;
>  			}
>  
> @@ -152,11 +153,13 @@ int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
>  				     &padint, shift);
>  			if (ret < 0) {
>  				ret = -errno;
> -				printf("failed to pass scan type description\n");
> +				fprintf(stderr,
> +					"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");
> +				fprintf(stderr,
> +					"scan type description didn't match\n");
>  				goto error_close_sysfsfp;
>  			}
>  
> @@ -170,7 +173,8 @@ int iioutils_get_type(unsigned *is_signed, unsigned *bytes, unsigned *bits_used,
>  			*is_signed = (signchar == 's');
>  			if (fclose(sysfsfp)) {
>  				ret = -errno;
> -				printf("Failed to close %s\n", filename);
> +				fprintf(stderr, "Failed to close %s\n",
> +					filename);
>  				goto error_free_filename;
>  			}
>  
> @@ -454,7 +458,8 @@ int build_channel_array(const char *device_dir,
>  			sysfsfp = fopen(filename, "r");
>  			if (!sysfsfp) {
>  				ret = -errno;
> -				printf("failed to open %s\n", filename);
> +				fprintf(stderr, "failed to open %s\n",
> +					filename);
>  				free(filename);
>  				goto error_cleanup_array;
>  			}
> @@ -568,7 +573,7 @@ int find_type_by_name(const char *name, const char *type)
>  
>  	dp = opendir(iio_dir);
>  	if (!dp) {
> -		printf("No industrialio devices available\n");
> +		fprintf(stderr, "No industrialio devices available\n");
>  		return -ENODEV;
>  	}
>  
> @@ -581,11 +586,13 @@ int find_type_by_name(const char *name, const char *type)
>  			ret = sscanf(ent->d_name + strlen(type), "%d", &number);
>  			if (ret < 0) {
>  				ret = -errno;
> -				printf("failed to read element number\n");
> +				fprintf(stderr,
> +					"failed to read element number\n");
>  				goto error_close_dir;
>  			} else if (ret != 1) {
>  				ret = -EIO;
> -				printf("failed to match element number\n");
> +				fprintf(stderr,
> +					"failed to match element number\n");
>  				goto error_close_dir;
>  			}
>  
> @@ -664,7 +671,7 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
>  	sysfsfp = fopen(temp, "w");
>  	if (!sysfsfp) {
>  		ret = -errno;
> -		printf("failed to open %s\n", temp);
> +		fprintf(stderr, "failed to open %s\n", temp);
>  		goto error_free;
>  	}
>  
> @@ -685,7 +692,7 @@ static int _write_sysfs_int(const char *filename, const char *basedir, int val,
>  		sysfsfp = fopen(temp, "r");
>  		if (!sysfsfp) {
>  			ret = -errno;
> -			printf("failed to open %s\n", temp);
> +			fprintf(stderr, "failed to open %s\n", temp);
>  			goto error_free;
>  		}
>  
> @@ -703,8 +710,9 @@ 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);
> +			fprintf(stderr,
> +				"Possible failure in int write %d to %s/%s\n",
> +				val, basedir, filename);
>  			ret = -1;
>  		}
>  	}
> @@ -750,7 +758,7 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
>  	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
>  
>  	if (!temp) {
> -		printf("Memory allocation failed\n");
> +		fprintf(stderr, "Memory allocation failed\n");
>  		return -ENOMEM;
>  	}
>  
> @@ -761,7 +769,7 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
>  	sysfsfp = fopen(temp, "w");
>  	if (!sysfsfp) {
>  		ret = -errno;
> -		printf("Could not open %s\n", temp);
> +		fprintf(stderr, "Could not open %s\n", temp);
>  		goto error_free;
>  	}
>  
> @@ -782,7 +790,7 @@ static int _write_sysfs_string(const char *filename, const char *basedir,
>  		sysfsfp = fopen(temp, "r");
>  		if (!sysfsfp) {
>  			ret = -errno;
> -			printf("Could not open file to verify\n");
> +			fprintf(stderr, "Could not open file to verify\n");
>  			goto error_free;
>  		}
>  
> @@ -800,9 +808,10 @@ 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);
> +			fprintf(stderr,
> +				"Possible failure in string write of %s "
> +				"Should be %s written to %s/%s\n", temp, val,
> +				basedir, filename);
>  			ret = -1;
>  		}
>  	}
> @@ -856,7 +865,7 @@ int read_sysfs_posint(const char *filename, const char *basedir)
>  	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
>  
>  	if (!temp) {
> -		printf("Memory allocation failed");
> +		fprintf(stderr, "Memory allocation failed");
>  		return -ENOMEM;
>  	}
>  
> @@ -903,7 +912,7 @@ int read_sysfs_float(const char *filename, const char *basedir, float *val)
>  	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
>  
>  	if (!temp) {
> -		printf("Memory allocation failed");
> +		fprintf(stderr, "Memory allocation failed");
>  		return -ENOMEM;
>  	}
>  
> @@ -950,7 +959,7 @@ int read_sysfs_string(const char *filename, const char *basedir, char *str)
>  	char *temp = malloc(strlen(basedir) + strlen(filename) + 2);
>  
>  	if (!temp) {
> -		printf("Memory allocation failed");
> +		fprintf(stderr, "Memory allocation failed");
>  		return -ENOMEM;
>  	}
>  
> diff --git a/tools/iio/lsiio.c b/tools/iio/lsiio.c
> index 4f8172f..b271a9a 100644
> --- a/tools/iio/lsiio.c
> +++ b/tools/iio/lsiio.c
> @@ -108,7 +108,7 @@ static int dump_devices(void)
>  
>  	dp = opendir(iio_dir);
>  	if (!dp) {
> -		printf("No industrial I/O devices available\n");
> +		fprintf(stderr, "No industrial I/O devices available\n");
>  		return -ENODEV;
>  	}
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists