[PATCH v2 1/2] tools: iio: iio_generic_buffer: fix types to match

[PATCH v2 1/2] tools: iio: iio_generic_buffer: fix types to match

[Martin Kelly]

Several types are mismatched and causing implicit conversions.  Fix them
up so the types match.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
 tools/iio/iio_generic_buffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index f0c6f54a8b2f..aa765c11992b 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -334,7 +334,10 @@ int main(int argc, char **argv)
 	unsigned long timedelay = 1000000;
 	unsigned long buf_len = 128;
 
-	int ret, c, i, j, toread;
+	ssize_t i;
+	unsigned long j;
+	unsigned long toread;
+	int ret, c;
 	int fp = -1;
 
 	int num_channels = 0;
-- 
2.11.0

[PATCH v2 2/2] tools: iio: iio_generic_buffer: allow continuous looping

[Martin Kelly]

Sometimes it's useful to stream samples forever, such as when
stress-testing a driver overnight to check for memory leaks or other
issues. When the program receives a signal, it will gracefully cleanup,
so it is still safe to terminate at any time.

Add support for specifying a negative -c option, meaning that we should
loop forever. To do so, we need to use a long long (instead of just
long) for num_loops so that current code specifying num_loops greater
than UNSIGNED_LONG_MAX doesn't break.

Signed-off-by: Martin Kelly <mkelly@xevo.com>
---
v2:
- Make j unsigned long long to match num_loops.

Another option for implementing this would be to implicitly assume looping
forever if -c is not specified (instead of the current default of 2). I did not
do this out of fear of breaking existing users but would be OK with doing so, as
I like it as a default. If you'd like to go this route, let me know.

 tools/iio/iio_generic_buffer.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
index aa765c11992b..3040830d7797 100644
--- a/tools/iio/iio_generic_buffer.c
+++ b/tools/iio/iio_generic_buffer.c
@@ -248,7 +248,7 @@ void print_usage(void)
 		"Capture, convert and output data from IIO device buffer\n"
 		"  -a         Auto-activate all available channels\n"
 		"  -A         Force-activate ALL channels\n"
-		"  -c <n>     Do n conversions\n"
+		"  -c <n>     Do n conversions, or loop forever if n < 0\n"
 		"  -e         Disable wait for event (new data)\n"
 		"  -g         Use trigger-less mode\n"
 		"  -l <n>     Set buffer length to n samples\n"
@@ -330,12 +330,12 @@ static const struct option longopts[] = {
 
 int main(int argc, char **argv)
 {
-	unsigned long num_loops = 2;
+	unsigned long long num_loops = 2;
 	unsigned long timedelay = 1000000;
 	unsigned long buf_len = 128;
 
 	ssize_t i;
-	unsigned long j;
+	unsigned long long j;
 	unsigned long toread;
 	int ret, c;
 	int fp = -1;
@@ -369,7 +369,7 @@ int main(int argc, char **argv)
 			break;	
 		case 'c':
 			errno = 0;
-			num_loops = strtoul(optarg, &dummy, 10);
+			num_loops = strtoll(optarg, &dummy, 10);
 			if (errno) {
 				ret = -errno;
 				goto error;
@@ -637,7 +637,7 @@ int main(int argc, char **argv)
 		goto error;
 	}
 
-	for (j = 0; j < num_loops; j++) {
+	for (j = 0; j < num_loops || num_loops < 0; j++) {
 		if (!noevents) {
 			struct pollfd pfd = {
 				.fd = fp,
-- 
2.11.0

Re: [PATCH v2 1/2] tools: iio: iio_generic_buffer: fix types to match

[Jonathan Cameron]

On Thu, 17 May 2018 17:14:45 -0700
Martin Kelly <mkelly@xevo.com> wrote:

> Several types are mismatched and causing implicit conversions.  Fix them
> up so the types match.
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>
Whilst I find it hard to care much about these, there might be a possible
bug as a result, so fair enough to clean them up.

Applied to the togreg branch of iio.git and pushed out as testing.

Jonathan
> ---
>  tools/iio/iio_generic_buffer.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index f0c6f54a8b2f..aa765c11992b 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -334,7 +334,10 @@ int main(int argc, char **argv)
>  	unsigned long timedelay = 1000000;
>  	unsigned long buf_len = 128;
>  
> -	int ret, c, i, j, toread;
> +	ssize_t i;
> +	unsigned long j;
> +	unsigned long toread;
> +	int ret, c;
>  	int fp = -1;
>  
>  	int num_channels = 0;

Re: [PATCH v2 2/2] tools: iio: iio_generic_buffer: allow continuous looping

[Jonathan Cameron]

On Thu, 17 May 2018 17:14:46 -0700
Martin Kelly <mkelly@xevo.com> wrote:

> Sometimes it's useful to stream samples forever, such as when
> stress-testing a driver overnight to check for memory leaks or other
> issues. When the program receives a signal, it will gracefully cleanup,
> so it is still safe to terminate at any time.
> 
> Add support for specifying a negative -c option, meaning that we should
> loop forever. To do so, we need to use a long long (instead of just
> long) for num_loops so that current code specifying num_loops greater
> than UNSIGNED_LONG_MAX doesn't break.
> 
> Signed-off-by: Martin Kelly <mkelly@xevo.com>

Seems reasonable and I agree this is better than defaulting to infinite looping.

Definitely likely to be test scripts out there relying on it terminating when
the count isn't specified.

Applied to the togreg branch of iio.git and pushed out as testing for the
autobuilders to ignore it.

Thanks,

Jonathan

> ---
> v2:
> - Make j unsigned long long to match num_loops.
> 
> Another option for implementing this would be to implicitly assume looping
> forever if -c is not specified (instead of the current default of 2). I did not
> do this out of fear of breaking existing users but would be OK with doing so, as
> I like it as a default. If you'd like to go this route, let me know.
> 
>  tools/iio/iio_generic_buffer.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/iio/iio_generic_buffer.c b/tools/iio/iio_generic_buffer.c
> index aa765c11992b..3040830d7797 100644
> --- a/tools/iio/iio_generic_buffer.c
> +++ b/tools/iio/iio_generic_buffer.c
> @@ -248,7 +248,7 @@ void print_usage(void)
>  		"Capture, convert and output data from IIO device buffer\n"
>  		"  -a         Auto-activate all available channels\n"
>  		"  -A         Force-activate ALL channels\n"
> -		"  -c <n>     Do n conversions\n"
> +		"  -c <n>     Do n conversions, or loop forever if n < 0\n"
>  		"  -e         Disable wait for event (new data)\n"
>  		"  -g         Use trigger-less mode\n"
>  		"  -l <n>     Set buffer length to n samples\n"
> @@ -330,12 +330,12 @@ static const struct option longopts[] = {
>  
>  int main(int argc, char **argv)
>  {
> -	unsigned long num_loops = 2;
> +	unsigned long long num_loops = 2;
>  	unsigned long timedelay = 1000000;
>  	unsigned long buf_len = 128;
>  
>  	ssize_t i;
> -	unsigned long j;
> +	unsigned long long j;
>  	unsigned long toread;
>  	int ret, c;
>  	int fp = -1;
> @@ -369,7 +369,7 @@ int main(int argc, char **argv)
>  			break;	
>  		case 'c':
>  			errno = 0;
> -			num_loops = strtoul(optarg, &dummy, 10);
> +			num_loops = strtoll(optarg, &dummy, 10);
>  			if (errno) {
>  				ret = -errno;
>  				goto error;
> @@ -637,7 +637,7 @@ int main(int argc, char **argv)
>  		goto error;
>  	}
>  
> -	for (j = 0; j < num_loops; j++) {
> +	for (j = 0; j < num_loops || num_loops < 0; j++) {
>  		if (!noevents) {
>  			struct pollfd pfd = {
>  				.fd = fp,

Re: [PATCH v2 1/2] tools: iio: iio_generic_buffer: fix types to match

[Martin Kelly]

On 05/20/2018 06:55 AM, Jonathan Cameron wrote:
> On Thu, 17 May 2018 17:14:45 -0700
> Martin Kelly <mkelly@xevo.com> wrote:
> 
>> Several types are mismatched and causing implicit conversions.  Fix them
>> up so the types match.
>>
>> Signed-off-by: Martin Kelly <mkelly@xevo.com>
> Whilst I find it hard to care much about these, there might be a possible
> bug as a result, so fair enough to clean them up.
> 
> Applied to the togreg branch of iio.git and pushed out as testing.
> 
> Jonathan

Indeed, I doubt this will cause problems, but it can't hurt. This was a 
"while I was at it" fix for patch 2, which was the change I really 
wanted :).