Re: [PATCH] split: adding --exec, --exec-wait, and --pause

[Chris Frey]

Hi folks,

I haven't seen any comments on this patch yet.  What can I do to help get
this patch included in coreutils?

Thanks,
- Chris


On Wed, Sep 14, 2005 at 08:14:07AM -0400, Chris Frey wrote:
> Hi,
> 
> This is an initial rough draft of a patch to add exec and pause support to
> the split command.
> 
> The basic idea is to let the user have some control of what happens to
> each output file as it is created.  A command may be run (say burning
> file chunks to CD), or split can pause for the user to press ENTER
> after each output file is closed.
> 
> The new help options look like this:
> 
>   -e, --exec=CMD          run CMD after each output file is closed
>   -w, --exec-wait=CMD     run CMD after each output file is closed
>                           and wait for the child to exit
>   -p, --pause             pause for keypress after each output file is closed
> 
> I'm not happy with the futzing around with the STDIN_FILENO, as both exec
> and pause need to read from the user's terminal, not the data stream.
> What I've done is check if STDOUT_FILENO is a tty, and if so, open() it.
> This causes problems if you su to another user and your tty is owned
> by the first user, but I'm not sure how to get around that.
> 
> This patch is in the public domain.
> 
> Let me know what you think,
> - Chris
> 
> 
> --- split.c   2005-09-14 08:00:12.000000000 -0400
> +++ split-cdf01.c     2005-09-14 07:59:48.000000000 -0400
> @@ -76,6 +76,17 @@
>     output file is opened. */
>  static bool verbose;
>  
> +/* If true, pause after each output file is closed, to let the user press
> +   ENTER.  Use tty_pause_fd for the I/O.  */
> +static bool pause_mode;
> +static int tty_pause_fd;
> +
> +/* The user-specified command to run after each output file is closed.
> +   Run it through sprintf first in order to place the latest output
> +   filename in the command */
> +static char *exec_command;
> +static bool exec_wait;
> +
>  /* For long options that have no equivalent short option, use a
>     non-character as a pseudo short option, starting with CHAR_MAX + 1.  */
>  enum
> @@ -91,6 +102,9 @@
>    {"suffix-length", required_argument, NULL, 'a'},
>    {"numeric-suffixes", no_argument, NULL, 'd'},
>    {"verbose", no_argument, NULL, VERBOSE_OPTION},
> +  {"pause", no_argument, NULL, 'p'},
> +  {"exec", required_argument, NULL, 'e'},
> +  {"exec-wait", required_argument, NULL, 'w'},
>    {GETOPT_HELP_OPTION_DECL},
>    {GETOPT_VERSION_OPTION_DECL},
>    {NULL, 0, NULL, 0}
> @@ -122,7 +136,11 @@
>    -b, --bytes=SIZE        put SIZE bytes per output file\n\
>    -C, --line-bytes=SIZE   put at most SIZE bytes of lines per output file\n\
>    -d, --numeric-suffixes  use numeric suffixes instead of alphabetic\n\
> +  -e, --exec=CMD          run CMD after each output file is closed\n\
> +  -w, --exec-wait=CMD     run CMD after each output file is closed\n\
> +                          and wait for the child to exit\n\
>    -l, --lines=NUMBER      put NUMBER lines per output file\n\
> +  -p, --pause             pause for keypress after each output file is 
> closed\n\
>  "), DEFAULT_SUFFIX_LENGTH);
>        fputs (_("\
>        --verbose           print a diagnostic to standard error just\n\
> @@ -195,6 +213,81 @@
>      }
>  }
>  
> +
> +static void
> +run_exec_command (void)
> +{
> +  int child_pid = fork();
> +  
> +  if (child_pid == 0)
> +    {
> +      /* child process */
> +      char *cmd_line;
> +      char tty_name[100];
> +      int stdin_fd;
> +
> +      /* close all data files */
> +      close (STDIN_FILENO);
> +      if (output_desc >= 0)
> +        close (output_desc);
> +      if (pause_mode)
> +        close (tty_pause_fd);
> +
> +      /* recreate a stdin for the child */
> +      if (ttyname_r(STDOUT_FILENO, tty_name, sizeof(tty_name)) != 0)
> +     error (EXIT_FAILURE, errno, "can't discover tty name");
> +      stdin_fd = open (tty_name, O_RDWR);
> +      if (stdin_fd == -1)
> +     error (EXIT_FAILURE, errno, "can't open tty for "
> +             "pause prompting");
> +      if (stdin_fd != STDIN_FILENO)
> +        {
> +       dup2 (stdin_fd, STDIN_FILENO);
> +       close (stdin_fd);
> +     }
> +
> +      /* build the command line, passing a few filenames so the user
> +       * can use multiple %s if he needs to... the cheap way */
> +      asprintf (&cmd_line, exec_command, outfile, outfile, outfile, outfile,
> +               outfile, outfile, outfile);
> +
> +      /* exec it */
> +      execl ("/bin/sh", "/bin/sh", "-c", cmd_line, NULL);
> +
> +      /* exit with failure if we get here */
> +      exit (1);
> +    }
> +  else if (child_pid == -1)
> +    {
> +      /* parent process, child had problems */
> +      error (EXIT_FAILURE, errno, "can't fork to exec command");
> +    }
> +  else
> +    {
> +      /* parent process, success */
> +      if (exec_wait)
> +     waitpid (child_pid, NULL, 0);
> +    }
> +}
> +
> +
> +static void
> +split_point (void)
> +{
> +  if (exec_command)
> +    run_exec_command ();
> +
> +  if (pause_mode)
> +    {
> +      char nothing;
> +      write(tty_pause_fd, "Press ENTER to continue with next split file\n"
> +            "Just finished: ", 60);
> +      write(tty_pause_fd, outfile, strlen(outfile));
> +      read(tty_pause_fd, &nothing, 1);
> +    }
> +}
> +
> +
>  /* Write BYTES bytes at BP to an output file.
>     If NEW_FILE_FLAG is true, open the next output file.
>     Otherwise add to the same output file already in use.  */
> @@ -206,6 +299,8 @@
>      {
>        if (output_desc >= 0 && close (output_desc) < 0)
>       error (EXIT_FAILURE, errno, "%s", outfile);
> +      else if (output_desc >= 0)
> +        split_point ();
>  
>        next_file_name ();
>        if (verbose)
> @@ -406,7 +501,7 @@
>        /* This is the argv-index of the option we will read next.  */
>        int this_optind = optind ? optind : 1;
>  
> -      c = getopt_long (argc, argv, "0123456789C:a:b:dl:", longopts, NULL);
> +      c = getopt_long (argc, argv, "0123456789C:a:b:dl:pe:w:", longopts, 
> NULL);
>        if (c == -1)
>       break;
>  
> @@ -494,6 +589,43 @@
>         suffix_alphabet = "0123456789";
>         break;
>  
> +     case 'p':
> +       pause_mode = true;
> +       if (isatty(STDOUT_FILENO))
> +         {
> +           /* we have a valid tty on stdout, let's try to open it for
> +            * reading and writing (for the prompt), since stdin is
> +            * being used for data */
> +           char tty_name[100];
> +           if (ttyname_r(STDOUT_FILENO, tty_name, sizeof(tty_name)) != 0)
> +             error (EXIT_FAILURE, errno, "can't discover tty name");
> +           tty_pause_fd = open (tty_name, O_RDWR);
> +           if (tty_pause_fd == -1)
> +             error (EXIT_FAILURE, errno, "can't open tty for "
> +                     "pause prompting");
> +         }
> +       else
> +         {
> +           error (EXIT_FAILURE, 0, "stdout is not a tty, can't prompt"
> +                  " on pause points");
> +         }
> +       break;
> +
> +     case 'e':
> +     case 'w':
> +       if (! exec_command)
> +         {
> +           size_t cmd_length = strlen(optarg);
> +           exec_command = xmalloc (cmd_length + 1);
> +           strcpy (exec_command, optarg);
> +
> +           if (c == 'w')
> +             exec_wait = true;
> +         }
> +       else
> +         error (EXIT_FAILURE, 0, "exec already specified!");
> +       break;
> +
>       case VERBOSE_OPTION:
>         verbose = true;
>         break;
> @@ -578,6 +710,8 @@
>      error (EXIT_FAILURE, errno, "%s", infile);
>    if (output_desc >= 0 && close (output_desc) < 0)
>      error (EXIT_FAILURE, errno, "%s", outfile);
> +  else if (output_desc >= 0)
> +    split_point ();
>  
>    exit (EXIT_SUCCESS);
>  }
>