* [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals
@ 2016-05-23 16:07 Crestez Dan Leonard
2016-05-23 16:07 ` [PATCH v2 2/2] iio: generic_buffer: Add --device-num option Crestez Dan Leonard
2016-05-29 19:14 ` [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Crestez Dan Leonard @ 2016-05-23 16:07 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard
This will clean (disable buffer/trigger/channels) when doing
something like a CTRL-C. Otherwise restarting generic_buffer requires a
manual echo 0 > buffer/enable
This also cleanup up all the code freeing string buffers at
the end of main. We initialize all pointers to NULL so that cleanup can
all be done under a single error label.
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
tools/iio/generic_buffer.c | 170 ++++++++++++++++++++++++++++-----------------
1 file changed, 107 insertions(+), 63 deletions(-)
diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 2429c78..53fd28b 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -32,6 +32,8 @@
#include <endian.h>
#include <getopt.h>
#include <inttypes.h>
+#include <stdbool.h>
+#include <signal.h>
#include "iio_utils.h"
/**
@@ -254,6 +256,65 @@ void print_usage(void)
" -w <n> Set delay between reads in us (event-less mode)\n");
}
+enum autochan autochannels = AUTOCHANNELS_DISABLED;
+char *dev_dir_name = NULL;
+char *buf_dir_name = NULL;
+bool current_trigger_set = false;
+
+void cleanup(void)
+{
+ int ret;
+
+ /* Disable trigger */
+ if (dev_dir_name && current_trigger_set) {
+ /* Disconnect the trigger - just write a dummy name. */
+ ret = write_sysfs_string("trigger/current_trigger",
+ dev_dir_name, "NULL");
+ if (ret < 0)
+ fprintf(stderr, "Failed to disable trigger: %s\n",
+ strerror(-ret));
+ current_trigger_set = false;
+ }
+
+ /* Disable buffer */
+ if (buf_dir_name) {
+ ret = write_sysfs_int("enable", buf_dir_name, 0);
+ if (ret < 0)
+ fprintf(stderr, "Failed to disable buffer: %s\n",
+ strerror(-ret));
+ }
+
+ /* Disable channels if auto-enabled */
+ if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
+ ret = enable_disable_all_channels(dev_dir_name, 0);
+ if (ret)
+ fprintf(stderr, "Failed to disable all channels\n");
+ autochannels = AUTOCHANNELS_DISABLED;
+ }
+}
+
+void sig_handler(int signum)
+{
+ fprintf(stderr, "Caught signal %d\n", signum);
+ cleanup();
+ exit(-signum);
+}
+
+void register_cleanup(void)
+{
+ struct sigaction sa = { .sa_handler = sig_handler };
+ const int signums[] = { SIGINT, SIGTERM, SIGABRT };
+ int ret, i;
+
+ for (i = 0; i < ARRAY_SIZE(signums); ++i) {
+ ret = sigaction(signums[i], &sa, NULL);
+ if (ret) {
+ perror("Failed to register signal handler");
+ exit(-1);
+ }
+ }
+}
+
int main(int argc, char **argv)
{
unsigned long num_loops = 2;
@@ -261,25 +322,24 @@ int main(int argc, char **argv)
unsigned long buf_len = 128;
int ret, c, i, j, toread;
- int fp;
+ int fp = -1;
- int num_channels;
+ int num_channels = 0;
char *trigger_name = NULL, *device_name = NULL;
- char *dev_dir_name, *buf_dir_name;
- int datardytrigger = 1;
char *data;
ssize_t read_size;
int dev_num, trig_num;
- char *buffer_access;
+ char *buffer_access = NULL;
int scan_size;
int noevents = 0;
int notrigger = 0;
- enum autochan autochannels = AUTOCHANNELS_DISABLED;
char *dummy;
struct iio_channel_info *channels;
+ register_cleanup();
+
while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
switch (c) {
case 'a':
@@ -288,8 +348,10 @@ int main(int argc, char **argv)
case 'c':
errno = 0;
num_loops = strtoul(optarg, &dummy, 10);
- if (errno)
- return -errno;
+ if (errno) {
+ ret = -errno;
+ goto error;
+ }
break;
case 'e':
@@ -301,26 +363,30 @@ int main(int argc, char **argv)
case 'l':
errno = 0;
buf_len = strtoul(optarg, &dummy, 10);
- if (errno)
- return -errno;
+ if (errno) {
+ ret = -errno;
+ goto error;
+ }
break;
case 'n':
device_name = optarg;
break;
case 't':
- trigger_name = optarg;
- datardytrigger = 0;
+ trigger_name = strdup(optarg);
break;
case 'w':
errno = 0;
timedelay = strtoul(optarg, &dummy, 10);
- if (errno)
- return -errno;
+ if (errno) {
+ ret = -errno;
+ goto error;
+ }
break;
case '?':
print_usage();
- return -1;
+ ret = -1;
+ goto error;
}
}
@@ -334,14 +400,17 @@ int main(int argc, char **argv)
dev_num = find_type_by_name(device_name, "iio:device");
if (dev_num < 0) {
fprintf(stderr, "Failed to find the %s\n", device_name);
- return dev_num;
+ ret = dev_num;
+ goto error;
}
printf("iio device number being used is %d\n", dev_num);
ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
- if (ret < 0)
- return -ENOMEM;
+ if (ret < 0) {
+ ret = -ENOMEM;
+ goto error;
+ }
if (!notrigger) {
if (!trigger_name) {
@@ -354,7 +423,7 @@ int main(int argc, char **argv)
"%s-dev%d", device_name, dev_num);
if (ret < 0) {
ret = -ENOMEM;
- goto error_free_dev_dir_name;
+ goto error;
}
}
@@ -367,7 +436,7 @@ int main(int argc, char **argv)
"%s-trigger", device_name);
if (ret < 0) {
ret = -ENOMEM;
- goto error_free_dev_dir_name;
+ goto error;
}
}
@@ -376,7 +445,7 @@ int main(int argc, char **argv)
fprintf(stderr, "Failed to find the trigger %s\n",
trigger_name);
ret = trig_num;
- goto error_free_triggername;
+ goto error;
}
printf("iio trigger number being used is %d\n", trig_num);
@@ -392,7 +461,7 @@ int main(int argc, char **argv)
if (ret) {
fprintf(stderr, "Problem reading scan element information\n"
"diag %s\n", dev_dir_name);
- goto error_free_triggername;
+ goto error;
}
if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
fprintf(stderr, "Auto-channels selected but some channels "
@@ -407,7 +476,7 @@ int main(int argc, char **argv)
ret = enable_disable_all_channels(dev_dir_name, 1);
if (ret) {
fprintf(stderr, "Failed to enable all channels\n");
- goto error_free_triggername;
+ goto error;
}
/* This flags that we need to disable the channels again */
@@ -419,12 +488,12 @@ int main(int argc, char **argv)
fprintf(stderr, "Problem reading scan element "
"information\n"
"diag %s\n", dev_dir_name);
- goto error_disable_channels;
+ goto error;
}
if (!num_channels) {
fprintf(stderr, "Still no channels after "
"auto-enabling, giving up\n");
- goto error_disable_channels;
+ goto error;
}
}
@@ -436,7 +505,7 @@ int main(int argc, char **argv)
"/*_en or pass -a to autoenable channels and "
"try again.\n", dev_dir_name);
ret = -ENOENT;
- goto error_free_triggername;
+ goto error;
}
/*
@@ -448,7 +517,7 @@ int main(int argc, char **argv)
"%siio:device%d/buffer", iio_dir, dev_num);
if (ret < 0) {
ret = -ENOMEM;
- goto error_free_channels;
+ goto error;
}
if (!notrigger) {
@@ -463,34 +532,34 @@ int main(int argc, char **argv)
if (ret < 0) {
fprintf(stderr,
"Failed to write current_trigger file\n");
- goto error_free_buf_dir_name;
+ goto error;
}
}
/* Setup ring buffer parameters */
ret = write_sysfs_int("length", buf_dir_name, buf_len);
if (ret < 0)
- goto error_free_buf_dir_name;
+ goto error;
/* Enable the buffer */
ret = write_sysfs_int("enable", buf_dir_name, 1);
if (ret < 0) {
fprintf(stderr,
"Failed to enable buffer: %s\n", strerror(-ret));
- goto error_free_buf_dir_name;
+ goto error;
}
scan_size = size_from_channelarray(channels, num_channels);
data = malloc(scan_size * buf_len);
if (!data) {
ret = -ENOMEM;
- goto error_free_buf_dir_name;
+ goto error;
}
ret = asprintf(&buffer_access, "/dev/iio:device%d", dev_num);
if (ret < 0) {
ret = -ENOMEM;
- goto error_free_data;
+ goto error;
}
/* Attempt to open non blocking the access dev */
@@ -498,7 +567,7 @@ int main(int argc, char **argv)
if (fp == -1) { /* TODO: If it isn't there make the node */
ret = -errno;
fprintf(stderr, "Failed to open %s\n", buffer_access);
- goto error_free_buffer_access;
+ goto error;
}
for (j = 0; j < num_loops; j++) {
@@ -511,7 +580,7 @@ int main(int argc, char **argv)
ret = poll(&pfd, 1, -1);
if (ret < 0) {
ret = -errno;
- goto error_close_buffer_access;
+ goto error;
} else if (ret == 0) {
continue;
}
@@ -536,45 +605,20 @@ int main(int argc, char **argv)
num_channels);
}
- /* Stop the buffer */
- ret = write_sysfs_int("enable", buf_dir_name, 0);
- if (ret < 0)
- goto error_close_buffer_access;
-
- if (!notrigger)
- /* Disconnect the trigger - just write a dummy name. */
- ret = write_sysfs_string("trigger/current_trigger",
- dev_dir_name, "NULL");
- if (ret < 0)
- fprintf(stderr, "Failed to write to %s\n",
- dev_dir_name);
+error:
+ cleanup();
-error_close_buffer_access:
- if (close(fp) == -1)
+ if (fp >= 0 && close(fp) == -1)
perror("Failed to close buffer");
-
-error_free_buffer_access:
free(buffer_access);
-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);
-error_disable_channels:
- if (autochannels == AUTOCHANNELS_ACTIVE) {
- ret = enable_disable_all_channels(dev_dir_name, 0);
- if (ret)
- fprintf(stderr, "Failed to disable all channels\n");
- }
-error_free_dev_dir_name:
+ free(trigger_name);
free(dev_dir_name);
return ret;
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] iio: generic_buffer: Add --device-num option
2016-05-23 16:07 [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Crestez Dan Leonard
@ 2016-05-23 16:07 ` Crestez Dan Leonard
2016-05-29 19:17 ` Jonathan Cameron
2016-05-29 19:14 ` [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Crestez Dan Leonard @ 2016-05-23 16:07 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta, Crestez Dan Leonard
This makes it possible to distinguish between iio devices with the same
name.
Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
---
tools/iio/generic_buffer.c | 69 ++++++++++++++++++++++++++++++++++------------
1 file changed, 51 insertions(+), 18 deletions(-)
diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
index 53fd28b..a93b771 100644
--- a/tools/iio/generic_buffer.c
+++ b/tools/iio/generic_buffer.c
@@ -251,7 +251,9 @@ void print_usage(void)
" -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"
+ " --device-name -n <name>\n"
+ " --device-num -N <num>\n"
+ " Set device by name or number"
" -t <name> Set trigger name\n"
" -w <n> Set delay between reads in us (event-less mode)\n");
}
@@ -315,6 +317,12 @@ void register_cleanup(void)
}
}
+static const struct option longopts[] = {
+ { "device-name", 1, 0, 'n' },
+ { "device-num", 1, 0, 'N' },
+ { },
+};
+
int main(int argc, char **argv)
{
unsigned long num_loops = 2;
@@ -329,7 +337,7 @@ int main(int argc, char **argv)
char *data;
ssize_t read_size;
- int dev_num, trig_num;
+ int dev_num = -1, trig_num;
char *buffer_access = NULL;
int scan_size;
int noevents = 0;
@@ -340,7 +348,7 @@ int main(int argc, char **argv)
register_cleanup();
- while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
+ while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) != -1) {
switch (c) {
case 'a':
autochannels = AUTOCHANNELS_ENABLED;
@@ -370,7 +378,15 @@ int main(int argc, char **argv)
break;
case 'n':
- device_name = optarg;
+ device_name = strdup(optarg);
+ break;
+ case 'N':
+ errno = 0;
+ dev_num = strtoul(optarg, &dummy, 10);
+ if (errno) {
+ ret = -errno;
+ goto error;
+ }
break;
case 't':
trigger_name = strdup(optarg);
@@ -390,26 +406,42 @@ int main(int argc, char **argv)
}
}
- if (!device_name) {
- fprintf(stderr, "Device name not set\n");
- print_usage();
- return -1;
- }
-
/* Find the device requested */
- dev_num = find_type_by_name(device_name, "iio:device");
- if (dev_num < 0) {
- fprintf(stderr, "Failed to find the %s\n", device_name);
- ret = dev_num;
+ if (dev_num < 0 && !device_name) {
+ fprintf(stderr, "Device not set\n");
+ print_usage();
+ ret = -1;
+ goto error;
+ } else if (dev_num >= 0 && device_name) {
+ fprintf(stderr, "Only one of --device-num or --device-name needs to be set\n");
+ print_usage();
+ ret = -1;
goto error;
+ } else if (dev_num < 0) {
+ dev_num = find_type_by_name(device_name, "iio:device");
+ if (dev_num < 0) {
+ fprintf(stderr, "Failed to find the %s\n", device_name);
+ ret = dev_num;
+ goto error;
+ }
}
-
printf("iio device number being used is %d\n", dev_num);
ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
- if (ret < 0) {
- ret = -ENOMEM;
- goto error;
+ if (ret < 0)
+ return -ENOMEM;
+ /* Fetch device_name if specified by number */
+ if (!device_name) {
+ device_name = malloc(IIO_MAX_NAME_LENGTH);
+ if (!device_name) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ ret = read_sysfs_string("name", dev_dir_name, device_name);
+ if (ret < 0) {
+ fprintf(stderr, "Failed to read name of device %d\n", dev_num);
+ goto error;
+ }
}
if (!notrigger) {
@@ -619,6 +651,7 @@ error:
}
free(channels);
free(trigger_name);
+ free(device_name);
free(dev_dir_name);
return ret;
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] iio: generic_buffer: Add --device-num option
2016-05-23 16:07 ` [PATCH v2 2/2] iio: generic_buffer: Add --device-num option Crestez Dan Leonard
@ 2016-05-29 19:17 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2016-05-29 19:17 UTC (permalink / raw)
To: Crestez Dan Leonard, linux-iio
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta
On 23/05/16 17:07, Crestez Dan Leonard wrote:
> This makes it possible to distinguish between iio devices with the same
> name.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
I'm a cheapskate and never had more than one of a given part ;)
Hence clearly never thought this through properly.
Applied to the togreg branch of iio.git - usual route via testing etc...
Thanks,
Jonathan
> ---
> tools/iio/generic_buffer.c | 69 ++++++++++++++++++++++++++++++++++------------
> 1 file changed, 51 insertions(+), 18 deletions(-)
>
> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
> index 53fd28b..a93b771 100644
> --- a/tools/iio/generic_buffer.c
> +++ b/tools/iio/generic_buffer.c
> @@ -251,7 +251,9 @@ void print_usage(void)
> " -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"
> + " --device-name -n <name>\n"
> + " --device-num -N <num>\n"
> + " Set device by name or number"
> " -t <name> Set trigger name\n"
> " -w <n> Set delay between reads in us (event-less mode)\n");
> }
> @@ -315,6 +317,12 @@ void register_cleanup(void)
> }
> }
>
> +static const struct option longopts[] = {
> + { "device-name", 1, 0, 'n' },
> + { "device-num", 1, 0, 'N' },
> + { },
> +};
> +
> int main(int argc, char **argv)
> {
> unsigned long num_loops = 2;
> @@ -329,7 +337,7 @@ int main(int argc, char **argv)
>
> char *data;
> ssize_t read_size;
> - int dev_num, trig_num;
> + int dev_num = -1, trig_num;
> char *buffer_access = NULL;
> int scan_size;
> int noevents = 0;
> @@ -340,7 +348,7 @@ int main(int argc, char **argv)
>
> register_cleanup();
>
> - while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
> + while ((c = getopt_long(argc, argv, "ac:egl:n:N:t:w:", longopts, NULL)) != -1) {
> switch (c) {
> case 'a':
> autochannels = AUTOCHANNELS_ENABLED;
> @@ -370,7 +378,15 @@ int main(int argc, char **argv)
>
> break;
> case 'n':
> - device_name = optarg;
> + device_name = strdup(optarg);
> + break;
> + case 'N':
> + errno = 0;
> + dev_num = strtoul(optarg, &dummy, 10);
> + if (errno) {
> + ret = -errno;
> + goto error;
> + }
> break;
> case 't':
> trigger_name = strdup(optarg);
> @@ -390,26 +406,42 @@ int main(int argc, char **argv)
> }
> }
>
> - if (!device_name) {
> - fprintf(stderr, "Device name not set\n");
> - print_usage();
> - return -1;
> - }
> -
> /* Find the device requested */
> - dev_num = find_type_by_name(device_name, "iio:device");
> - if (dev_num < 0) {
> - fprintf(stderr, "Failed to find the %s\n", device_name);
> - ret = dev_num;
> + if (dev_num < 0 && !device_name) {
> + fprintf(stderr, "Device not set\n");
> + print_usage();
> + ret = -1;
> + goto error;
> + } else if (dev_num >= 0 && device_name) {
> + fprintf(stderr, "Only one of --device-num or --device-name needs to be set\n");
> + print_usage();
> + ret = -1;
> goto error;
> + } else if (dev_num < 0) {
> + dev_num = find_type_by_name(device_name, "iio:device");
> + if (dev_num < 0) {
> + fprintf(stderr, "Failed to find the %s\n", device_name);
> + ret = dev_num;
> + goto error;
> + }
> }
> -
> printf("iio device number being used is %d\n", dev_num);
>
> ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
> - if (ret < 0) {
> - ret = -ENOMEM;
> - goto error;
> + if (ret < 0)
> + return -ENOMEM;
> + /* Fetch device_name if specified by number */
> + if (!device_name) {
> + device_name = malloc(IIO_MAX_NAME_LENGTH);
> + if (!device_name) {
> + ret = -ENOMEM;
> + goto error;
> + }
> + ret = read_sysfs_string("name", dev_dir_name, device_name);
> + if (ret < 0) {
> + fprintf(stderr, "Failed to read name of device %d\n", dev_num);
> + goto error;
> + }
> }
>
> if (!notrigger) {
> @@ -619,6 +651,7 @@ error:
> }
> free(channels);
> free(trigger_name);
> + free(device_name);
> free(dev_dir_name);
>
> return ret;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals
2016-05-23 16:07 [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Crestez Dan Leonard
2016-05-23 16:07 ` [PATCH v2 2/2] iio: generic_buffer: Add --device-num option Crestez Dan Leonard
@ 2016-05-29 19:14 ` Jonathan Cameron
2016-05-29 19:15 ` Jonathan Cameron
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2016-05-29 19:14 UTC (permalink / raw)
To: Crestez Dan Leonard, linux-iio
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta
On 23/05/16 17:07, Crestez Dan Leonard wrote:
> This will clean (disable buffer/trigger/channels) when doing
> something like a CTRL-C. Otherwise restarting generic_buffer requires a
> manual echo 0 > buffer/enable
>
> This also cleanup up all the code freeing string buffers at
> the end of main. We initialize all pointers to NULL so that cleanup can
> all be done under a single error label.
>
> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
>From a testing point of view, this is really nice to have.
Thanks Leonard!
Jonathan
> ---
> tools/iio/generic_buffer.c | 170 ++++++++++++++++++++++++++++-----------------
> 1 file changed, 107 insertions(+), 63 deletions(-)
>
> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
> index 2429c78..53fd28b 100644
> --- a/tools/iio/generic_buffer.c
> +++ b/tools/iio/generic_buffer.c
> @@ -32,6 +32,8 @@
> #include <endian.h>
> #include <getopt.h>
> #include <inttypes.h>
> +#include <stdbool.h>
> +#include <signal.h>
> #include "iio_utils.h"
>
> /**
> @@ -254,6 +256,65 @@ void print_usage(void)
> " -w <n> Set delay between reads in us (event-less mode)\n");
> }
>
> +enum autochan autochannels = AUTOCHANNELS_DISABLED;
> +char *dev_dir_name = NULL;
> +char *buf_dir_name = NULL;
> +bool current_trigger_set = false;
> +
> +void cleanup(void)
> +{
> + int ret;
> +
> + /* Disable trigger */
> + if (dev_dir_name && current_trigger_set) {
> + /* Disconnect the trigger - just write a dummy name. */
> + ret = write_sysfs_string("trigger/current_trigger",
> + dev_dir_name, "NULL");
> + if (ret < 0)
> + fprintf(stderr, "Failed to disable trigger: %s\n",
> + strerror(-ret));
> + current_trigger_set = false;
> + }
> +
> + /* Disable buffer */
> + if (buf_dir_name) {
> + ret = write_sysfs_int("enable", buf_dir_name, 0);
> + if (ret < 0)
> + fprintf(stderr, "Failed to disable buffer: %s\n",
> + strerror(-ret));
> + }
> +
> + /* Disable channels if auto-enabled */
> + if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
> + ret = enable_disable_all_channels(dev_dir_name, 0);
> + if (ret)
> + fprintf(stderr, "Failed to disable all channels\n");
> + autochannels = AUTOCHANNELS_DISABLED;
> + }
> +}
> +
> +void sig_handler(int signum)
> +{
> + fprintf(stderr, "Caught signal %d\n", signum);
> + cleanup();
> + exit(-signum);
> +}
> +
> +void register_cleanup(void)
> +{
> + struct sigaction sa = { .sa_handler = sig_handler };
> + const int signums[] = { SIGINT, SIGTERM, SIGABRT };
> + int ret, i;
> +
> + for (i = 0; i < ARRAY_SIZE(signums); ++i) {
> + ret = sigaction(signums[i], &sa, NULL);
> + if (ret) {
> + perror("Failed to register signal handler");
> + exit(-1);
> + }
> + }
> +}
> +
> int main(int argc, char **argv)
> {
> unsigned long num_loops = 2;
> @@ -261,25 +322,24 @@ int main(int argc, char **argv)
> unsigned long buf_len = 128;
>
> int ret, c, i, j, toread;
> - int fp;
> + int fp = -1;
>
> - int num_channels;
> + int num_channels = 0;
> char *trigger_name = NULL, *device_name = NULL;
> - char *dev_dir_name, *buf_dir_name;
>
> - int datardytrigger = 1;
> char *data;
> ssize_t read_size;
> int dev_num, trig_num;
> - char *buffer_access;
> + char *buffer_access = NULL;
> int scan_size;
> int noevents = 0;
> int notrigger = 0;
> - enum autochan autochannels = AUTOCHANNELS_DISABLED;
> char *dummy;
>
> struct iio_channel_info *channels;
>
> + register_cleanup();
> +
> while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
> switch (c) {
> case 'a':
> @@ -288,8 +348,10 @@ int main(int argc, char **argv)
> case 'c':
> errno = 0;
> num_loops = strtoul(optarg, &dummy, 10);
> - if (errno)
> - return -errno;
> + if (errno) {
> + ret = -errno;
> + goto error;
> + }
>
> break;
> case 'e':
> @@ -301,26 +363,30 @@ int main(int argc, char **argv)
> case 'l':
> errno = 0;
> buf_len = strtoul(optarg, &dummy, 10);
> - if (errno)
> - return -errno;
> + if (errno) {
> + ret = -errno;
> + goto error;
> + }
>
> break;
> case 'n':
> device_name = optarg;
> break;
> case 't':
> - trigger_name = optarg;
> - datardytrigger = 0;
> + trigger_name = strdup(optarg);
> break;
> case 'w':
> errno = 0;
> timedelay = strtoul(optarg, &dummy, 10);
> - if (errno)
> - return -errno;
> + if (errno) {
> + ret = -errno;
> + goto error;
> + }
> break;
> case '?':
> print_usage();
> - return -1;
> + ret = -1;
> + goto error;
> }
> }
>
> @@ -334,14 +400,17 @@ int main(int argc, char **argv)
> dev_num = find_type_by_name(device_name, "iio:device");
> if (dev_num < 0) {
> fprintf(stderr, "Failed to find the %s\n", device_name);
> - return dev_num;
> + ret = dev_num;
> + goto error;
> }
>
> printf("iio device number being used is %d\n", dev_num);
>
> ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
> - if (ret < 0)
> - return -ENOMEM;
> + if (ret < 0) {
> + ret = -ENOMEM;
> + goto error;
> + }
>
> if (!notrigger) {
> if (!trigger_name) {
> @@ -354,7 +423,7 @@ int main(int argc, char **argv)
> "%s-dev%d", device_name, dev_num);
> if (ret < 0) {
> ret = -ENOMEM;
> - goto error_free_dev_dir_name;
> + goto error;
> }
> }
>
> @@ -367,7 +436,7 @@ int main(int argc, char **argv)
> "%s-trigger", device_name);
> if (ret < 0) {
> ret = -ENOMEM;
> - goto error_free_dev_dir_name;
> + goto error;
> }
> }
>
> @@ -376,7 +445,7 @@ int main(int argc, char **argv)
> fprintf(stderr, "Failed to find the trigger %s\n",
> trigger_name);
> ret = trig_num;
> - goto error_free_triggername;
> + goto error;
> }
>
> printf("iio trigger number being used is %d\n", trig_num);
> @@ -392,7 +461,7 @@ int main(int argc, char **argv)
> if (ret) {
> fprintf(stderr, "Problem reading scan element information\n"
> "diag %s\n", dev_dir_name);
> - goto error_free_triggername;
> + goto error;
> }
> if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
> fprintf(stderr, "Auto-channels selected but some channels "
> @@ -407,7 +476,7 @@ int main(int argc, char **argv)
> ret = enable_disable_all_channels(dev_dir_name, 1);
> if (ret) {
> fprintf(stderr, "Failed to enable all channels\n");
> - goto error_free_triggername;
> + goto error;
> }
>
> /* This flags that we need to disable the channels again */
> @@ -419,12 +488,12 @@ int main(int argc, char **argv)
> fprintf(stderr, "Problem reading scan element "
> "information\n"
> "diag %s\n", dev_dir_name);
> - goto error_disable_channels;
> + goto error;
> }
> if (!num_channels) {
> fprintf(stderr, "Still no channels after "
> "auto-enabling, giving up\n");
> - goto error_disable_channels;
> + goto error;
> }
> }
>
> @@ -436,7 +505,7 @@ int main(int argc, char **argv)
> "/*_en or pass -a to autoenable channels and "
> "try again.\n", dev_dir_name);
> ret = -ENOENT;
> - goto error_free_triggername;
> + goto error;
> }
>
> /*
> @@ -448,7 +517,7 @@ int main(int argc, char **argv)
> "%siio:device%d/buffer", iio_dir, dev_num);
> if (ret < 0) {
> ret = -ENOMEM;
> - goto error_free_channels;
> + goto error;
> }
>
> if (!notrigger) {
> @@ -463,34 +532,34 @@ int main(int argc, char **argv)
> if (ret < 0) {
> fprintf(stderr,
> "Failed to write current_trigger file\n");
> - goto error_free_buf_dir_name;
> + goto error;
> }
> }
>
> /* Setup ring buffer parameters */
> ret = write_sysfs_int("length", buf_dir_name, buf_len);
> if (ret < 0)
> - goto error_free_buf_dir_name;
> + goto error;
>
> /* Enable the buffer */
> ret = write_sysfs_int("enable", buf_dir_name, 1);
> if (ret < 0) {
> fprintf(stderr,
> "Failed to enable buffer: %s\n", strerror(-ret));
> - goto error_free_buf_dir_name;
> + goto error;
> }
>
> scan_size = size_from_channelarray(channels, num_channels);
> data = malloc(scan_size * buf_len);
> if (!data) {
> ret = -ENOMEM;
> - goto error_free_buf_dir_name;
> + goto error;
> }
>
> ret = asprintf(&buffer_access, "/dev/iio:device%d", dev_num);
> if (ret < 0) {
> ret = -ENOMEM;
> - goto error_free_data;
> + goto error;
> }
>
> /* Attempt to open non blocking the access dev */
> @@ -498,7 +567,7 @@ int main(int argc, char **argv)
> if (fp == -1) { /* TODO: If it isn't there make the node */
> ret = -errno;
> fprintf(stderr, "Failed to open %s\n", buffer_access);
> - goto error_free_buffer_access;
> + goto error;
> }
>
> for (j = 0; j < num_loops; j++) {
> @@ -511,7 +580,7 @@ int main(int argc, char **argv)
> ret = poll(&pfd, 1, -1);
> if (ret < 0) {
> ret = -errno;
> - goto error_close_buffer_access;
> + goto error;
> } else if (ret == 0) {
> continue;
> }
> @@ -536,45 +605,20 @@ int main(int argc, char **argv)
> num_channels);
> }
>
> - /* Stop the buffer */
> - ret = write_sysfs_int("enable", buf_dir_name, 0);
> - if (ret < 0)
> - goto error_close_buffer_access;
> -
> - if (!notrigger)
> - /* Disconnect the trigger - just write a dummy name. */
> - ret = write_sysfs_string("trigger/current_trigger",
> - dev_dir_name, "NULL");
> - if (ret < 0)
> - fprintf(stderr, "Failed to write to %s\n",
> - dev_dir_name);
> +error:
> + cleanup();
>
> -error_close_buffer_access:
> - if (close(fp) == -1)
> + if (fp >= 0 && close(fp) == -1)
> perror("Failed to close buffer");
> -
> -error_free_buffer_access:
> free(buffer_access);
> -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);
> -error_disable_channels:
> - if (autochannels == AUTOCHANNELS_ACTIVE) {
> - ret = enable_disable_all_channels(dev_dir_name, 0);
> - if (ret)
> - fprintf(stderr, "Failed to disable all channels\n");
> - }
> -error_free_dev_dir_name:
> + free(trigger_name);
> free(dev_dir_name);
>
> return ret;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals
2016-05-29 19:14 ` [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Jonathan Cameron
@ 2016-05-29 19:15 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2016-05-29 19:15 UTC (permalink / raw)
To: Crestez Dan Leonard, linux-iio
Cc: linux-kernel, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Daniel Baluta
On 29/05/16 20:14, Jonathan Cameron wrote:
> On 23/05/16 17:07, Crestez Dan Leonard wrote:
>> This will clean (disable buffer/trigger/channels) when doing
>> something like a CTRL-C. Otherwise restarting generic_buffer requires a
>> manual echo 0 > buffer/enable
>>
>> This also cleanup up all the code freeing string buffers at
>> the end of main. We initialize all pointers to NULL so that cleanup can
>> all be done under a single error label.
>>
>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
> From a testing point of view, this is really nice to have.
>
> Thanks Leonard!
Oh yeah - applied with a bit of frigging to follow the name change
that crossed with me getting around to looking at this. Should be in
togreg pushed as testing etc etc.
Thanks,
Jonathan
>
> Jonathan
>> ---
>> tools/iio/generic_buffer.c | 170 ++++++++++++++++++++++++++++-----------------
>> 1 file changed, 107 insertions(+), 63 deletions(-)
>>
>> diff --git a/tools/iio/generic_buffer.c b/tools/iio/generic_buffer.c
>> index 2429c78..53fd28b 100644
>> --- a/tools/iio/generic_buffer.c
>> +++ b/tools/iio/generic_buffer.c
>> @@ -32,6 +32,8 @@
>> #include <endian.h>
>> #include <getopt.h>
>> #include <inttypes.h>
>> +#include <stdbool.h>
>> +#include <signal.h>
>> #include "iio_utils.h"
>>
>> /**
>> @@ -254,6 +256,65 @@ void print_usage(void)
>> " -w <n> Set delay between reads in us (event-less mode)\n");
>> }
>>
>> +enum autochan autochannels = AUTOCHANNELS_DISABLED;
>> +char *dev_dir_name = NULL;
>> +char *buf_dir_name = NULL;
>> +bool current_trigger_set = false;
>> +
>> +void cleanup(void)
>> +{
>> + int ret;
>> +
>> + /* Disable trigger */
>> + if (dev_dir_name && current_trigger_set) {
>> + /* Disconnect the trigger - just write a dummy name. */
>> + ret = write_sysfs_string("trigger/current_trigger",
>> + dev_dir_name, "NULL");
>> + if (ret < 0)
>> + fprintf(stderr, "Failed to disable trigger: %s\n",
>> + strerror(-ret));
>> + current_trigger_set = false;
>> + }
>> +
>> + /* Disable buffer */
>> + if (buf_dir_name) {
>> + ret = write_sysfs_int("enable", buf_dir_name, 0);
>> + if (ret < 0)
>> + fprintf(stderr, "Failed to disable buffer: %s\n",
>> + strerror(-ret));
>> + }
>> +
>> + /* Disable channels if auto-enabled */
>> + if (dev_dir_name && autochannels == AUTOCHANNELS_ACTIVE) {
>> + ret = enable_disable_all_channels(dev_dir_name, 0);
>> + if (ret)
>> + fprintf(stderr, "Failed to disable all channels\n");
>> + autochannels = AUTOCHANNELS_DISABLED;
>> + }
>> +}
>> +
>> +void sig_handler(int signum)
>> +{
>> + fprintf(stderr, "Caught signal %d\n", signum);
>> + cleanup();
>> + exit(-signum);
>> +}
>> +
>> +void register_cleanup(void)
>> +{
>> + struct sigaction sa = { .sa_handler = sig_handler };
>> + const int signums[] = { SIGINT, SIGTERM, SIGABRT };
>> + int ret, i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(signums); ++i) {
>> + ret = sigaction(signums[i], &sa, NULL);
>> + if (ret) {
>> + perror("Failed to register signal handler");
>> + exit(-1);
>> + }
>> + }
>> +}
>> +
>> int main(int argc, char **argv)
>> {
>> unsigned long num_loops = 2;
>> @@ -261,25 +322,24 @@ int main(int argc, char **argv)
>> unsigned long buf_len = 128;
>>
>> int ret, c, i, j, toread;
>> - int fp;
>> + int fp = -1;
>>
>> - int num_channels;
>> + int num_channels = 0;
>> char *trigger_name = NULL, *device_name = NULL;
>> - char *dev_dir_name, *buf_dir_name;
>>
>> - int datardytrigger = 1;
>> char *data;
>> ssize_t read_size;
>> int dev_num, trig_num;
>> - char *buffer_access;
>> + char *buffer_access = NULL;
>> int scan_size;
>> int noevents = 0;
>> int notrigger = 0;
>> - enum autochan autochannels = AUTOCHANNELS_DISABLED;
>> char *dummy;
>>
>> struct iio_channel_info *channels;
>>
>> + register_cleanup();
>> +
>> while ((c = getopt(argc, argv, "ac:egl:n:t:w:")) != -1) {
>> switch (c) {
>> case 'a':
>> @@ -288,8 +348,10 @@ int main(int argc, char **argv)
>> case 'c':
>> errno = 0;
>> num_loops = strtoul(optarg, &dummy, 10);
>> - if (errno)
>> - return -errno;
>> + if (errno) {
>> + ret = -errno;
>> + goto error;
>> + }
>>
>> break;
>> case 'e':
>> @@ -301,26 +363,30 @@ int main(int argc, char **argv)
>> case 'l':
>> errno = 0;
>> buf_len = strtoul(optarg, &dummy, 10);
>> - if (errno)
>> - return -errno;
>> + if (errno) {
>> + ret = -errno;
>> + goto error;
>> + }
>>
>> break;
>> case 'n':
>> device_name = optarg;
>> break;
>> case 't':
>> - trigger_name = optarg;
>> - datardytrigger = 0;
>> + trigger_name = strdup(optarg);
>> break;
>> case 'w':
>> errno = 0;
>> timedelay = strtoul(optarg, &dummy, 10);
>> - if (errno)
>> - return -errno;
>> + if (errno) {
>> + ret = -errno;
>> + goto error;
>> + }
>> break;
>> case '?':
>> print_usage();
>> - return -1;
>> + ret = -1;
>> + goto error;
>> }
>> }
>>
>> @@ -334,14 +400,17 @@ int main(int argc, char **argv)
>> dev_num = find_type_by_name(device_name, "iio:device");
>> if (dev_num < 0) {
>> fprintf(stderr, "Failed to find the %s\n", device_name);
>> - return dev_num;
>> + ret = dev_num;
>> + goto error;
>> }
>>
>> printf("iio device number being used is %d\n", dev_num);
>>
>> ret = asprintf(&dev_dir_name, "%siio:device%d", iio_dir, dev_num);
>> - if (ret < 0)
>> - return -ENOMEM;
>> + if (ret < 0) {
>> + ret = -ENOMEM;
>> + goto error;
>> + }
>>
>> if (!notrigger) {
>> if (!trigger_name) {
>> @@ -354,7 +423,7 @@ int main(int argc, char **argv)
>> "%s-dev%d", device_name, dev_num);
>> if (ret < 0) {
>> ret = -ENOMEM;
>> - goto error_free_dev_dir_name;
>> + goto error;
>> }
>> }
>>
>> @@ -367,7 +436,7 @@ int main(int argc, char **argv)
>> "%s-trigger", device_name);
>> if (ret < 0) {
>> ret = -ENOMEM;
>> - goto error_free_dev_dir_name;
>> + goto error;
>> }
>> }
>>
>> @@ -376,7 +445,7 @@ int main(int argc, char **argv)
>> fprintf(stderr, "Failed to find the trigger %s\n",
>> trigger_name);
>> ret = trig_num;
>> - goto error_free_triggername;
>> + goto error;
>> }
>>
>> printf("iio trigger number being used is %d\n", trig_num);
>> @@ -392,7 +461,7 @@ int main(int argc, char **argv)
>> if (ret) {
>> fprintf(stderr, "Problem reading scan element information\n"
>> "diag %s\n", dev_dir_name);
>> - goto error_free_triggername;
>> + goto error;
>> }
>> if (num_channels && autochannels == AUTOCHANNELS_ENABLED) {
>> fprintf(stderr, "Auto-channels selected but some channels "
>> @@ -407,7 +476,7 @@ int main(int argc, char **argv)
>> ret = enable_disable_all_channels(dev_dir_name, 1);
>> if (ret) {
>> fprintf(stderr, "Failed to enable all channels\n");
>> - goto error_free_triggername;
>> + goto error;
>> }
>>
>> /* This flags that we need to disable the channels again */
>> @@ -419,12 +488,12 @@ int main(int argc, char **argv)
>> fprintf(stderr, "Problem reading scan element "
>> "information\n"
>> "diag %s\n", dev_dir_name);
>> - goto error_disable_channels;
>> + goto error;
>> }
>> if (!num_channels) {
>> fprintf(stderr, "Still no channels after "
>> "auto-enabling, giving up\n");
>> - goto error_disable_channels;
>> + goto error;
>> }
>> }
>>
>> @@ -436,7 +505,7 @@ int main(int argc, char **argv)
>> "/*_en or pass -a to autoenable channels and "
>> "try again.\n", dev_dir_name);
>> ret = -ENOENT;
>> - goto error_free_triggername;
>> + goto error;
>> }
>>
>> /*
>> @@ -448,7 +517,7 @@ int main(int argc, char **argv)
>> "%siio:device%d/buffer", iio_dir, dev_num);
>> if (ret < 0) {
>> ret = -ENOMEM;
>> - goto error_free_channels;
>> + goto error;
>> }
>>
>> if (!notrigger) {
>> @@ -463,34 +532,34 @@ int main(int argc, char **argv)
>> if (ret < 0) {
>> fprintf(stderr,
>> "Failed to write current_trigger file\n");
>> - goto error_free_buf_dir_name;
>> + goto error;
>> }
>> }
>>
>> /* Setup ring buffer parameters */
>> ret = write_sysfs_int("length", buf_dir_name, buf_len);
>> if (ret < 0)
>> - goto error_free_buf_dir_name;
>> + goto error;
>>
>> /* Enable the buffer */
>> ret = write_sysfs_int("enable", buf_dir_name, 1);
>> if (ret < 0) {
>> fprintf(stderr,
>> "Failed to enable buffer: %s\n", strerror(-ret));
>> - goto error_free_buf_dir_name;
>> + goto error;
>> }
>>
>> scan_size = size_from_channelarray(channels, num_channels);
>> data = malloc(scan_size * buf_len);
>> if (!data) {
>> ret = -ENOMEM;
>> - goto error_free_buf_dir_name;
>> + goto error;
>> }
>>
>> ret = asprintf(&buffer_access, "/dev/iio:device%d", dev_num);
>> if (ret < 0) {
>> ret = -ENOMEM;
>> - goto error_free_data;
>> + goto error;
>> }
>>
>> /* Attempt to open non blocking the access dev */
>> @@ -498,7 +567,7 @@ int main(int argc, char **argv)
>> if (fp == -1) { /* TODO: If it isn't there make the node */
>> ret = -errno;
>> fprintf(stderr, "Failed to open %s\n", buffer_access);
>> - goto error_free_buffer_access;
>> + goto error;
>> }
>>
>> for (j = 0; j < num_loops; j++) {
>> @@ -511,7 +580,7 @@ int main(int argc, char **argv)
>> ret = poll(&pfd, 1, -1);
>> if (ret < 0) {
>> ret = -errno;
>> - goto error_close_buffer_access;
>> + goto error;
>> } else if (ret == 0) {
>> continue;
>> }
>> @@ -536,45 +605,20 @@ int main(int argc, char **argv)
>> num_channels);
>> }
>>
>> - /* Stop the buffer */
>> - ret = write_sysfs_int("enable", buf_dir_name, 0);
>> - if (ret < 0)
>> - goto error_close_buffer_access;
>> -
>> - if (!notrigger)
>> - /* Disconnect the trigger - just write a dummy name. */
>> - ret = write_sysfs_string("trigger/current_trigger",
>> - dev_dir_name, "NULL");
>> - if (ret < 0)
>> - fprintf(stderr, "Failed to write to %s\n",
>> - dev_dir_name);
>> +error:
>> + cleanup();
>>
>> -error_close_buffer_access:
>> - if (close(fp) == -1)
>> + if (fp >= 0 && close(fp) == -1)
>> perror("Failed to close buffer");
>> -
>> -error_free_buffer_access:
>> free(buffer_access);
>> -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);
>> -error_disable_channels:
>> - if (autochannels == AUTOCHANNELS_ACTIVE) {
>> - ret = enable_disable_all_channels(dev_dir_name, 0);
>> - if (ret)
>> - fprintf(stderr, "Failed to disable all channels\n");
>> - }
>> -error_free_dev_dir_name:
>> + free(trigger_name);
>> free(dev_dir_name);
>>
>> return ret;
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-29 19:17 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-23 16:07 [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Crestez Dan Leonard
2016-05-23 16:07 ` [PATCH v2 2/2] iio: generic_buffer: Add --device-num option Crestez Dan Leonard
2016-05-29 19:17 ` Jonathan Cameron
2016-05-29 19:14 ` [PATCH v2 1/2] iio: generic_buffer: Cleanup when receiving signals Jonathan Cameron
2016-05-29 19:15 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).