Linux-IIO Archive on lore.kernel.org
 help / Atom feed
* [bug report] tools: iio: iio_generic_buffer: allow continuous looping
@ 2019-01-08 20:38 Dan Carpenter
  2019-01-08 22:20 ` Martin Kelly
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2019-01-08 20:38 UTC (permalink / raw)
  To: mkelly; +Cc: linux-iio

Hello Martin Kelly,

The patch 55dda0abcf9d: "tools: iio: iio_generic_buffer: allow
continuous looping" from May 17, 2018, leads to the following static
checker warning:

	./tools/iio/iio_generic_buffer.c:640 main()
	warn: unsigned 'num_loops' is never less than zero.

./tools/iio/iio_generic_buffer.c
    331 int main(int argc, char **argv)
    332 {
    333 	unsigned long long num_loops = 2;
                ^^^^^^^^

    334 	unsigned long timedelay = 1000000;
    335 	unsigned long buf_len = 128;
    336 
    337 	ssize_t i;
    338 	unsigned long long j;
    339 	unsigned long toread;
    340 	int ret, c;
    341 	int fp = -1;
    342 
    343 	int num_channels = 0;
    344 	char *trigger_name = NULL, *device_name = NULL;
    345 
    346 	char *data = NULL;
    347 	ssize_t read_size;
    348 	int dev_num = -1, trig_num = -1;
    349 	char *buffer_access = NULL;
    350 	int scan_size;
    351 	int noevents = 0;
    352 	int notrigger = 0;
    353 	char *dummy;
    354 	bool force_autochannels = false;
    355 
    356 	struct iio_channel_info *channels = NULL;
    357 

[ snip ]

    632 	/* Attempt to open non blocking the access dev */
    633 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
    634 	if (fp == -1) { /* TODO: If it isn't there make the node */
    635 		ret = -errno;
    636 		fprintf(stderr, "Failed to open %s\n", buffer_access);
    637 		goto error;
    638 	}
    639 
--> 640 	for (j = 0; j < num_loops || num_loops < 0; j++) {
                                             ^^^^^^^^^^^^^
If num_loops is -1 then it's supposed to loop forever.  It basically
does, except for the static checker warning.

    641 		if (!noevents) {
    642 			struct pollfd pfd = {
    643 				.fd = fp,
    644 				.events = POLLIN,
    645 			};
    646 
    647 			ret = poll(&pfd, 1, -1);
    648 			if (ret < 0) {
    649 				ret = -errno;
    650 				goto error;
    651 			} else if (ret == 0) {
    652 				continue;
    653 			}
    654 
    655 			toread = buf_len;
    656 		} else {
    657 			usleep(timedelay);
    658 			toread = 64;
    659 		}

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [bug report] tools: iio: iio_generic_buffer: allow continuous looping
  2019-01-08 20:38 [bug report] tools: iio: iio_generic_buffer: allow continuous looping Dan Carpenter
@ 2019-01-08 22:20 ` Martin Kelly
  2019-01-09  6:23   ` Dan Carpenter
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Kelly @ 2019-01-08 22:20 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-iio

On 1/8/19 12:38 PM, Dan Carpenter wrote:
> Hello Martin Kelly,
> 
> The patch 55dda0abcf9d: "tools: iio: iio_generic_buffer: allow
> continuous looping" from May 17, 2018, leads to the following static
> checker warning:
> 
> 	./tools/iio/iio_generic_buffer.c:640 main()
> 	warn: unsigned 'num_loops' is never less than zero.
> 

Thanks for catching this. How can I repro this so that I can make sure 
the static analyzer is happy once I fix it?

> ./tools/iio/iio_generic_buffer.c
>      331 int main(int argc, char **argv)
>      332 {
>      333 	unsigned long long num_loops = 2;
>                  ^^^^^^^^
> 
>      334 	unsigned long timedelay = 1000000;
>      335 	unsigned long buf_len = 128;
>      336
>      337 	ssize_t i;
>      338 	unsigned long long j;
>      339 	unsigned long toread;
>      340 	int ret, c;
>      341 	int fp = -1;
>      342
>      343 	int num_channels = 0;
>      344 	char *trigger_name = NULL, *device_name = NULL;
>      345
>      346 	char *data = NULL;
>      347 	ssize_t read_size;
>      348 	int dev_num = -1, trig_num = -1;
>      349 	char *buffer_access = NULL;
>      350 	int scan_size;
>      351 	int noevents = 0;
>      352 	int notrigger = 0;
>      353 	char *dummy;
>      354 	bool force_autochannels = false;
>      355
>      356 	struct iio_channel_info *channels = NULL;
>      357
> 
> [ snip ]
> 
>      632 	/* Attempt to open non blocking the access dev */
>      633 	fp = open(buffer_access, O_RDONLY | O_NONBLOCK);
>      634 	if (fp == -1) { /* TODO: If it isn't there make the node */
>      635 		ret = -errno;
>      636 		fprintf(stderr, "Failed to open %s\n", buffer_access);
>      637 		goto error;
>      638 	}
>      639
> --> 640 	for (j = 0; j < num_loops || num_loops < 0; j++) {
>                                               ^^^^^^^^^^^^^
> If num_loops is -1 then it's supposed to loop forever.  It basically
> does, except for the static checker warning.
> 

Yep, looks like -1 gets converted to ULONG_LONG_MAX and it loops for a 
very long time but not forever. I will fix it soon.

>      641 		if (!noevents) {
>      642 			struct pollfd pfd = {
>      643 				.fd = fp,
>      644 				.events = POLLIN,
>      645 			};
>      646
>      647 			ret = poll(&pfd, 1, -1);
>      648 			if (ret < 0) {
>      649 				ret = -errno;
>      650 				goto error;
>      651 			} else if (ret == 0) {
>      652 				continue;
>      653 			}
>      654
>      655 			toread = buf_len;
>      656 		} else {
>      657 			usleep(timedelay);
>      658 			toread = 64;
>      659 		}
> 
> regards,
> dan carpenter
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [bug report] tools: iio: iio_generic_buffer: allow continuous looping
  2019-01-08 22:20 ` Martin Kelly
@ 2019-01-09  6:23   ` Dan Carpenter
  2019-01-11 23:03     ` Martin Kelly
  0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2019-01-09  6:23 UTC (permalink / raw)
  To: Martin Kelly; +Cc: linux-iio

On Tue, Jan 08, 2019 at 10:20:38PM +0000, Martin Kelly wrote:
> On 1/8/19 12:38 PM, Dan Carpenter wrote:
> > Hello Martin Kelly,
> > 
> > The patch 55dda0abcf9d: "tools: iio: iio_generic_buffer: allow
> > continuous looping" from May 17, 2018, leads to the following static
> > checker warning:
> > 
> > 	./tools/iio/iio_generic_buffer.c:640 main()
> > 	warn: unsigned 'num_loops' is never less than zero.
> > 
> 
> Thanks for catching this. How can I repro this so that I can make sure 
> the static analyzer is happy once I fix it?
> 

It's a Smatch warning.  In this case you would just do:

~/path/to/smatch/smatch tools/iio/iio_generic_buffer.c

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [bug report] tools: iio: iio_generic_buffer: allow continuous looping
  2019-01-09  6:23   ` Dan Carpenter
@ 2019-01-11 23:03     ` Martin Kelly
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Kelly @ 2019-01-11 23:03 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-iio

On 1/8/19 10:23 PM, Dan Carpenter wrote>
> It's a Smatch warning.  In this case you would just do:
> 
> ~/path/to/smatch/smatch tools/iio/iio_generic_buffer.c
> 

Thanks, I sent a patch to fix the issue.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 20:38 [bug report] tools: iio: iio_generic_buffer: allow continuous looping Dan Carpenter
2019-01-08 22:20 ` Martin Kelly
2019-01-09  6:23   ` Dan Carpenter
2019-01-11 23:03     ` Martin Kelly

Linux-IIO Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iio/0 linux-iio/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iio linux-iio/ https://lore.kernel.org/linux-iio \
		linux-iio@vger.kernel.org linux-iio@archiver.kernel.org
	public-inbox-index linux-iio


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-iio


AGPL code for this site: git clone https://public-inbox.org/ public-inbox