I gave a quick look at replacing prompt with readpassphrase(3), but that
would be more trouble than it's worth. (adjusting pledge, semantics
change in where the "?..." would be printed).

Minor nits inline, but either way OK martijn@

On Thu, 2020-07-16 at 21:49 +0200, Ingo Schwarze wrote:
> Hi Jan,
> 
> Jan Stary wrote on Thu, Jul 16, 2020 at 09:45:44AM +0200:
> 
> > Does xargs need to set LC_MESSAGES?
> 
> As it stands, your patch doesn't make much sense.  It is true that
> it doesn't change behaviour, but it leaves more cruft behund than
> it tidies up.
> 
> That said, i agree that we will never implement LC_MESSAGES.
> 
> That allows a nice cleanup, simplifying the code and getting rid
> of several headers and several calls to complicated functions.
> 
> Remarks:
>  * .Tn was deprecated years ago.
>  * On OpenBSD, as the manual page says, nl_langinfo(3) is really
>    only useful for CODESET.
>  * There is no need to put a marker "return value ignored on
>    purpose" where ignoring the return value almost never indicates
>    a bug, like for fprintf, fflush, fclose.

If you're going to change this, please also remove them from run()

>  * Usually, we prefer getline(3) over fgetln(3), but none of the
>    arguments making it better really applies here, so we can as
>    well leave it.  The only downside is that ttyfp needs to remain
>    open until the stdio input buffer has been inspected, so we need
>    another automatic variable, "doit", but it's still simpler than
>    the malloc(3) dance that getline(3) would require.

Another option would be fgets, but probably still more hassle then
it's worth.

> OK?
>   Ingo
> 
> 
> Index: xargs.1
> ===================================================================
> RCS file: /cvs/src/usr.bin/xargs/xargs.1,v
> retrieving revision 1.28
> diff -u -p -r1.28 xargs.1
> --- xargs.1   4 Jun 2014 06:48:33 -0000       1.28
> +++ xargs.1   16 Jul 2020 19:29:52 -0000
> @@ -213,11 +213,11 @@ at once.
>  .It Fl p
>  Echo each command to be executed and ask the user whether it should be
>  executed.
> -An affirmative response,
> +If the answer starts with
>  .Ql y
> -in the POSIX locale,
> -causes the command to be executed, any other response causes it to be
> -skipped.
> +or
> +.Ql Y ,
> +the command is executed; otherwise it is skipped.
>  No commands are executed if the process is not attached to a terminal.
>  .It Fl R Ar replacements
>  Specify the maximum number of arguments that
> @@ -330,8 +330,7 @@ The flags
>  are extensions to
>  .St -p1003.1-2008 .
>  .Pp
> -The meanings of the 123, 124, and 125 exit values were taken from
> -.Tn GNU
> +The meanings of the 123, 124, and 125 exit values were taken from GNU
>  .Nm xargs .
>  .Sh HISTORY
>  The
> Index: xargs.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/xargs/xargs.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 xargs.c
> --- xargs.c   12 Jun 2018 15:24:31 -0000      1.34
> +++ xargs.c   16 Jul 2020 19:29:52 -0000
> @@ -41,10 +41,7 @@
>  #include <err.h>
>  #include <errno.h>
>  #include <fcntl.h>
> -#include <langinfo.h>
> -#include <locale.h>
>  #include <paths.h>
> -#include <regex.h>
>  #include <signal.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -86,8 +83,6 @@ main(int argc, char *argv[])
>       eofstr = "";
>       Jflag = nflag = 0;
>  
> -     (void)setlocale(LC_MESSAGES, "");
> -
>       /*
>        * POSIX.2 limits the exec line length to ARG_MAX - 2K.  Running that
>        * caused some E2BIG errors, so it was changed to ARG_MAX - 4K.  Given
> @@ -607,26 +602,19 @@ waitchildren(const char *name, int waita
>  static int
>  prompt(void)
>  {
> -     regex_t cre;
>       size_t rsize;
> -     int match;
>       char *response;
>       FILE *ttyfp;
> +     int doit = 0;
>  
>       if ((ttyfp = fopen(_PATH_TTY, "r")) == NULL)
>               return (2);     /* Indicate that the TTY failed to open. */
> -     (void)fprintf(stderr, "?...");
> -     (void)fflush(stderr);
> -     if ((response = fgetln(ttyfp, &rsize)) == NULL ||
> -         regcomp(&cre, nl_langinfo(YESEXPR), REG_BASIC) != 0) {
> -             (void)fclose(ttyfp);
> -             return (0);
> -     }
> -     response[rsize - 1] = '\0';
> -     match = regexec(&cre, response, 0, NULL, 0);
> -     (void)fclose(ttyfp);
> -     regfree(&cre);
> -     return (match == 0);
> +     fprintf(stderr, "?...");
> +     fflush(stderr);

According to setvbuf(3):
The standard error stream stderr is initially unbuffered.
And since setvbuf is not called in xargs this seems superfluous to me.
If this is correct, there's a second fflush that can also be removed.

> +     response = fgetln(ttyfp, &rsize);
> +     doit = response != NULL && (*response == 'y' || *response == 'Y');
> +     fclose(ttyfp);
> +     return (doit);
>  }
>  
>  static void
> 

Reply via email to