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

[Bartosz Golaszewski <brgl@xxxxxxxx>]

pon., 11 lut 2019 o 09:54 Martin Hundebøll <martin@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>

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
>