Re: [hackers] [sbase][PATCH 1/2] libutil: Implement a simple yes/no prompt

From: Tom Schwindl <schwindl_AT_posteo.de>
Date: Sun, 20 Nov 2022 14:58:53 +0000

Hi Alan,

On Sun Nov 20, 2022 at 1:17 PM CET, Alan Potteiger wrote:
> On Sun, Nov 20, 2022 at 2:17 AM Sebastian LaVine <mail_AT_smlavine.com> wrote:
> >
> > This only reads a single character, not a line as the standard states.
> > If the user responds with 'y\n', then that '\n' is going to be read if
> > the program takes any more input. I suggest using getline(3) instead.
> >
> > [0]: https://www.man7.org/linux/man-pages/man1/cp.1p.html
> >
>
> My mistake. I misunderstood how the buffering works in stdio, I played
> with it and
> saw exactly what you mean. Switched to getline(3).
>
> ---
> Makefile | 1 +
> libutil/promptyn.c | 26 ++++++++++++++++++++++++++
> util.h | 1 +
> 3 files changed, 28 insertions(+)
> create mode 100644 libutil/promptyn.c
>
> diff --git a/Makefile b/Makefile
> index 3243b1c..0a5c930 100644
> --- a/Makefile
> +++ b/Makefile
> _AT_@ -63,6 +63,7 @@ LIBUTILSRC =\
> libutil/mkdirp.c\
> libutil/mode.c\
> libutil/parseoffset.c\
> + libutil/promptyn.c\
> libutil/putword.c\
> libutil/reallocarray.c\
> libutil/recurse.c\
> diff --git a/libutil/promptyn.c b/libutil/promptyn.c
> new file mode 100644
> index 0000000..b63bfaa
> --- /dev/null
> +++ b/libutil/promptyn.c
> _AT_@ -0,0 +1,26 @@
> +/* See LICENSE file for copyright and license details. */
> +#include <stdio.h>
> +#include <stdarg.h>
> +
> +int
> +promptyn(const char *nomsg, const char *promptfmt, ...)
> +{
> + va_list ap;
> + char *linep = NULL;
> + size_t linecapp = 1;
> +
> + va_start(ap, promptfmt);
> + vfprintf(stderr, promptfmt, ap);
> + va_end(ap);
> +
> + fprintf(stderr, " (y/n [n]): ");
> + getline(&linep, &linecapp, stdin);
> + switch (*linep) {
> + case 'y': case 'Y':
> + return 0;
> + default:
> + fprintf(stderr, "%s\n", nomsg);
> + return 1;
> + }
> +}
> +

You should check if getline(3) failed before dereferencing the pointer.
linecapp should be zero (or not initialized) since its a throw away variable.
linep should be freed.

I don't quite get why there is a variable argument list instead of just using
a single argument which contains the prompt. I can't see the need for formatting
in a y/n prompt. I also wouldn't use `nomsg' since it just polutes stderr.
Additionally, you should fix your style in this diff.

I'm not sure if we even _want_ to implement the `-i' option.
Someone with more knowledge about the sbase design history could elaborate a bit
further on this.

--
Best Regards,
Tom Schwindl
Received on Sun Nov 20 2022 - 15:58:53 CET

This archive was generated by hypermail 2.3.0 : Sun Nov 20 2022 - 16:00:38 CET