[libgpiod,PATCHv2] gpiomon: add option to set line buffered output

Some applications call gpiomon in a sub process, in which case glibc
defaults to block buffered output on stdout. This makes the output
arrive to the calling process only when the (4kB) buffer is filled (or
when gpiomon exists), making the information obsolete and pretty much
useless.

Support such scenarios by adding a switch to configure line buffered
output on stdout. Similar switches are available in other applications
(e.g. `rsync`s --output argument).

Signed-off-by: Martin Hundebøll <martin@geanix.com>
---

Change since v1:
 * correct short argument in help text

 src/tools/gpiomon.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

pon., 11 lut 2019 o 09:54 Martin Hundebøll <martin@geanix.com> napisał(a):
>
> Some applications call gpiomon in a sub process, in which case glibc
> defaults to block buffered output on stdout. This makes the output
> arrive to the calling process only when the (4kB) buffer is filled (or
> when gpiomon exists), making the information obsolete and pretty much
> useless.
>
> Support such scenarios by adding a switch to configure line buffered
> output on stdout. Similar switches are available in other applications
> (e.g. `rsync`s --output argument).
>
> Signed-off-by: Martin Hundebøll <martin@geanix.com>
> ---
>

Hi Martin,

thanks for the patch, I like the idea. Unfortunately this patch
doesn't apply against current master. Can you check that?

> Change since v1:
>  * correct short argument in help text
>
>  src/tools/gpiomon.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/tools/gpiomon.c b/src/tools/gpiomon.c
> index c13ab38..1e2ba8e 100644
> --- a/src/tools/gpiomon.c
> +++ b/src/tools/gpiomon.c
> @@ -26,11 +26,12 @@ static const struct option longopts[] = {
>         { "silent",             no_argument,            NULL,   's' },
>         { "rising-edge",        no_argument,            NULL,   'r' },
>         { "falling-edge",       no_argument,            NULL,   'f' },
> +       { "linebuffered",       no_argument,            NULL,   'b' },
>         { "format",             required_argument,      NULL,   'F' },
>         { GETOPT_NULL_LONGOPT },
>  };
>
> -static const char *const shortopts = "+hvln:srfF:";
> +static const char *const shortopts = "+hvln:srfbF:";
>
>  static void print_help(void)
>  {
> @@ -46,6 +47,7 @@ static void print_help(void)
>         printf("  -s, --silent:\t\tdon't print event info\n");
>         printf("  -r, --rising-edge:\tonly process rising edge events\n");
>         printf("  -f, --falling-edge:\tonly process falling edge events\n");
> +       printf("  -b, --linebuffered:\tset line buffered output\n");
>         printf("  -F, --format=FMT\tspecify custom output format\n");
>         printf("\n");
>         printf("Format specifiers:\n");
> @@ -242,6 +244,7 @@ int main(int argc, char **argv)
>  {
>         unsigned int offsets[GPIOD_LINE_BULK_MAX_LINES], num_lines = 0, offset;
>         bool active_low = false, watch_rising = false, watch_falling = false;
> +       bool linebuffered = false;

Can you use a reverse christmas-tree ordering like everywhere else?
Preferably even add this to the line above if it fits in 80 columns.

Bart

>         struct timespec timeout = { 10, 0 };
>         int optc, opti, ret, i, event_type;
>         struct mon_ctx ctx;
> @@ -278,6 +281,9 @@ int main(int argc, char **argv)
>                 case 'f':
>                         watch_falling = true;
>                         break;
> +               case 'b':
> +                       linebuffered = true;
> +                       break;
>                 case 'F':
>                         ctx.fmt = optarg;
>                         break;
> @@ -298,6 +304,9 @@ int main(int argc, char **argv)
>         else
>                 event_type = GPIOD_CTXLESS_EVENT_BOTH_EDGES;
>
> +       if (linebuffered && setvbuf(stdout, NULL, _IOLBF, 0) != 0)
> +               die("failed to set line buffering");
> +
>         if (argc < 1)
>                 die("gpiochip must be specified");
>
> --
> 2.20.1
>

pon., 11 lut 2019 o 09:54 Martin Hundebøll <martin@geanix.com> napisał(a):
>
> Some applications call gpiomon in a sub process, in which case glibc
> defaults to block buffered output on stdout. This makes the output
> arrive to the calling process only when the (4kB) buffer is filled (or
> when gpiomon exists), making the information obsolete and pretty much
> useless.
>
> Support such scenarios by adding a switch to configure line buffered
> output on stdout. Similar switches are available in other applications
> (e.g. `rsync`s --output argument).
>
> Signed-off-by: Martin Hundebøll <martin@geanix.com>
> ---
>
> Change since v1:
>  * correct short argument in help text
>
>  src/tools/gpiomon.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/tools/gpiomon.c b/src/tools/gpiomon.c
> index c13ab38..1e2ba8e 100644
> --- a/src/tools/gpiomon.c
> +++ b/src/tools/gpiomon.c
> @@ -26,11 +26,12 @@ static const struct option longopts[] = {
>         { "silent",             no_argument,            NULL,   's' },
>         { "rising-edge",        no_argument,            NULL,   'r' },
>         { "falling-edge",       no_argument,            NULL,   'f' },
> +       { "linebuffered",       no_argument,            NULL,   'b' },
>         { "format",             required_argument,      NULL,   'F' },
>         { GETOPT_NULL_LONGOPT },
>  };
>
> -static const char *const shortopts = "+hvln:srfF:";
> +static const char *const shortopts = "+hvln:srfbF:";
>
>  static void print_help(void)
>  {
> @@ -46,6 +47,7 @@ static void print_help(void)
>         printf("  -s, --silent:\t\tdon't print event info\n");
>         printf("  -r, --rising-edge:\tonly process rising edge events\n");
>         printf("  -f, --falling-edge:\tonly process falling edge events\n");
> +       printf("  -b, --linebuffered:\tset line buffered output\n");
>         printf("  -F, --format=FMT\tspecify custom output format\n");
>         printf("\n");
>         printf("Format specifiers:\n");
> @@ -242,6 +244,7 @@ int main(int argc, char **argv)
>  {
>         unsigned int offsets[GPIOD_LINE_BULK_MAX_LINES], num_lines = 0, offset;
>         bool active_low = false, watch_rising = false, watch_falling = false;
> +       bool linebuffered = false;
>         struct timespec timeout = { 10, 0 };
>         int optc, opti, ret, i, event_type;
>         struct mon_ctx ctx;
> @@ -278,6 +281,9 @@ int main(int argc, char **argv)
>                 case 'f':
>                         watch_falling = true;
>                         break;
> +               case 'b':
> +                       linebuffered = true;
> +                       break;
>                 case 'F':
>                         ctx.fmt = optarg;
>                         break;
> @@ -298,6 +304,9 @@ int main(int argc, char **argv)
>         else
>                 event_type = GPIOD_CTXLESS_EVENT_BOTH_EDGES;
>
> +       if (linebuffered && setvbuf(stdout, NULL, _IOLBF, 0) != 0)
> +               die("failed to set line buffering");
> +

One more thing: setvbuf() sets errno on failure - can you use
die_perror()? Also: please assign the result of setvbuf() to ret and
check it in the subsequent line as is done everywhere else.

Bart

>         if (argc < 1)
>                 die("gpiochip must be specified");
>
> --
> 2.20.1
>