Re: [PATCH] Output prompt when stdin is not a TTY.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, May 31, 2023 at 12:52 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> On Wed, May 31, 2023 at 11:05 AM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
> >
> > On Wed, May 31, 2023 at 10:54 AM HAGIO KAZUHITO(萩尾 一仁)
> > <k-hagio-ab@xxxxxxx> wrote:
> > >
> > > On 2023/05/31 3:13, Hsin-Yi Wang wrote:
> > > > When stdin is not a TTY, prompt ("crash> ") won't be displayed. If
> > > > another process interact with crash with piped stdin/stdout, it will not
> > > > get the prompt as a delimiter.
> > > >
> > > > Compared to other debugger like gdb, crash seems intended to give a
> > > > prompt in this case in the beginning of process_command_line(). It
> > > > checks if pc->flags does NOT have any of
> > > > READLINE|SILENT|CMDLINE_IFILE|RCHOME_IFILE|RCLOCAL_IFILE, a
> > > > prompt should be printed. The check will never be true since READLINE is
> > > > set in setup_environment() unconditionally. It makes more sense to
> > > > change the READLINE flag in the check to TTY instead.
> > > >
> > > > Additionally, when stdin is not TTY, repeat the command line from user
> > > > after prompt, which can give more context.
> > > >
> > > > The prompt and command line can be opt out by using the silent (-s) flag.
> > > >
> > > > Signed-off-by: Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx>
> > >
> > > Thanks, but I found a case that prints two prompts, after a fatal error:
> > >
> > > # cat ~/.crashrc
> > > sys config        <<-- any command that fails with error(FATAL)
> > > sys
> > > # echo quit | crash
> > > ...
> > > 2 crash> sys config
> > > sys: kernel_config_data does not exist in this kernel
> > > 1 crash> 2 crash> sys
> > >        KERNEL: /usr/lib/debug/lib/modules/4.18.0-305.el8.x86_64/vmlinux
> > > ...
> > >
> > > where the crash has this:
> > >
> > > --- a/cmdline.c
> > > +++ b/cmdline.c
> > > @@ -66,7 +66,7 @@ process_command_line(void)
> > >
> > >          if (!(pc->flags &
> > >              (TTY|SILENT|CMDLINE_IFILE|RCHOME_IFILE|RCLOCAL_IFILE)))
> > > -               fprintf(fp, "%s", pc->prompt);
> > > +               fprintf(fp, "1 %s", pc->prompt);
> > >          fflush(fp);
> > >
> > >          /*
> > > @@ -1487,7 +1487,7 @@ exec_input_file(void)
> > >                          continue;
> > >
> > >                   if (!(pc->flags & SILENT)) {
> > > -                        fprintf(fp, "%s%s", pc->prompt, buf);
> > > +                        fprintf(fp, "2 %s%s", pc->prompt, buf);
>
> Ah, I misread this as the fprintf in process_command_line. I'll take a
> look at this issue to see how to avoid it.  Sorry for the confusion.
>
> >
> > I don't think we need to print a prompt again here. It can be used as
> > a delimiter when all the previous output are done and waiting for user
> > input. If we print the prompt again. what it will read:
> >
> > crash 8.0.3++
> > 1 crash>
> > // suppose stdin has cmd1 after here.
> > 2 crash> cmd1
> > cmd1_output
> > 1 crash>
> > // supposes stdin has cmd2 after here.
> > 2 crash> cmd2
> > cmd2_output
> >
> > Without the 2nd prompt:
> > crash 8.0.3++
> > 1 crash>
> > // suppose stdin has cmd1 after here.
> > cmd1
> > cmd1_output
> > 1 crash>
> > // suppose stdin has cmd2 after here.
> > cmd2
> > cmd2_output
> >
> > >                           fflush(fp);
> > >                   }
> > >
> > > Can we avoid this?
> > >

This is addressed in v2. In process_command_line(), we can tell if
we're still recovering from FATAL command in the input file, don't
print the prompt as it will be printed later.
Test cases:
1.
~/.crashrc
sys config
sys
output:
2 crash> sys config
sys: kernel_config_data does not exist in this kernel
2 crash> sys
      KERNEL:...
1 crash> quit

2.
~/.crashrc
sys
sys
output:
2 crash> sys
      KERNEL:...
2 crash> sys
      KERNEL:...
1 crash> quit

Thanks for catching this!

> > > Thanks,
> > > Kazu
> > >
> > > > ---
> > > > v1:
> > > > - The original discussion: https://listman.redhat.com/archives/crash-utility/2023-May/010710.html
> > > > - Kazu: provide the idea to print the command line as well.
> > > > ---
> > > >   cmdline.c | 6 +++++-
> > > >   1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/cmdline.c b/cmdline.c
> > > > index ded6551..9821d86 100644
> > > > --- a/cmdline.c
> > > > +++ b/cmdline.c
> > > > @@ -65,7 +65,7 @@ process_command_line(void)
> > > >       BZERO(pc->command_line, BUFSIZE);
> > > >
> > > >       if (!(pc->flags &
> > > > -         (READLINE|SILENT|CMDLINE_IFILE|RCHOME_IFILE|RCLOCAL_IFILE)))
> > > > +         (TTY|SILENT|CMDLINE_IFILE|RCHOME_IFILE|RCLOCAL_IFILE)))
> > > >               fprintf(fp, "%s", pc->prompt);
> > > >       fflush(fp);
> > > >
> > > > @@ -139,6 +139,10 @@ process_command_line(void)
> > > >           } else {
> > > >               if (fgets(pc->command_line, BUFSIZE-1, stdin) == NULL)
> > > >                       clean_exit(1);
> > > > +             if (!(pc->flags & SILENT)) {
> > > > +                     fprintf(fp, "%s", pc->command_line);
> > > > +                     fflush(fp);
> > > > +             }
> > > >               clean_line(pc->command_line);
> > > >               strcpy(pc->orig_line, pc->command_line);
> > > >           }

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux