On Tue, May 30, 2023 at 4:05 PM Hsin-Yi Wang <hsinyi@xxxxxxxxxxxx> wrote:
>
> On Tue, May 30, 2023 at 3:14 PM HAGIO KAZUHITO(萩尾 一仁)
> <k-hagio-ab@xxxxxxx> wrote:
> >
> > On 2023/05/30 14:09, Hsin-Yi Wang wrote:
> > > On Tue, May 30, 2023 at 10:31 AM HAGIO KAZUHITO(萩尾 一仁)
> > > <k-hagio-ab@xxxxxxx> wrote:
> > >>
> > >> On 2023/05/28 5:17, Hsin-Yi Wang wrote:
> > >>> hi crash-utility community,
> > >>>
> > >>> When stdin is not a TTY, but all the other flags remain the same,
> > >>> prompt ("crash> ") won't be displayed. An example use case is, the
> > >>> stdin of crash is replaced by a piped fd connected to another process.
> > >>>
> > >>> In process_command_line(), it checks if pc->flags does NOT have any of
> > >>> the flag: READLINE|SILENT|CMDLINE_IFILE|RCHOME_IFILE|RCLOCAL_IFILE, a
> > >>> prompt should be printed.
> > >>>
> > >>> But in setup_environment(), pc->flags is set to have READLINE flag[2],
> > >>> the above check will not be true at all.
> > >>>
> > >>> Should READLINE be set for all cases in setup_environment()?
> > >>> - If true, should the check in process_command_line() look for TTY
> > >>> instead of READLINE? Since if pc->flags has TTY, [2] won't be true and
> > >>> the prompt will be printed later in TTY's case[3].
> > >>> - If false, where should be a proper place and conditions to set READLINE?
> > >>>
> > >>> Or is the current behavior intended? I may not fully understand the
> > >>> design logic. Any explanations are appreciated.
> > >>
> > >> I don't know the full history of crash, but my impression of the flag is
> > >> that probably it's an ancient code and almost meaningless now, but the
> > >> current behavior is intended.
> > >>
> > >> What you want to do is displaying the prompt and command without a tty?
> > >
> > > Interact with crash through another process that piped stdin/stdout
> > > with crash to do additional input/output processing, so the filemode
> > > won't work.
> >
> > Ah, got it. So the prompt can be used as a delimiter or something? It
> > might be useful.
> >
>
> Right.
>
> > >
> > >> This is also my imagination, but probably they wanted only command
> > >> output like this without the prompt and command:
> > >>
> > >> # echo sys | crash -s
> > >> KERNEL: /usr/lib/debug/lib/modules/4.18.0-305.el8.x86_64/vmlinux
> > >> DUMPFILE: /proc/kcore
> > >> CPUS: 16
> > >> DATE: Tue May 30 11:15:41 JST 2023
> > >> UPTIME: 19 days, 23:23:48
> > >> LOAD AVERAGE: 0.18, 0.12, 0.09
> > >> TASKS: 555
> > >> NODENAME: r110j-1
> > >> RELEASE: 4.18.0-305.el8.x86_64
> > >> VERSION: #1 SMP Thu Apr 29 08:54:30 EDT 2021
> > >> MACHINE: x86_64 (3400 Mhz)
> > >> MEMORY: 63.9 GB
> > >> #
> > >
> > > But I wonder if the current behavior is intended, wouldn't [1] never
> > > be run because of [2]?
> > > [1] https://github.com/crash-utility/crash/blob/05a3a328fcd8920e49926b6d1c9c81ce0b6acbca/cmdline.c#L67
> > > [2] https://github.com/crash-utility/crash/blob/8246dce99dd23457e8c7a3fe9609c706694d1959/main.c#L1182
> >
> > Right, I guessed that it ultimately became so after changes, although it
> > should have got removed. But maybe not.
> >
> > >
> > > Another not necessarily related data point is, I compared the gdb tool
> > > and it will also give the prompt if no tty presented.
> > >
> > >>
> > >> Because if they wanted the prompt and command, it would be easily done
> > >> like this...
> > >>
> > >> --- a/cmdline.c
> > >> +++ b/cmdline.c
> > >> @@ -139,6 +139,7 @@ process_command_line(void)
> > >> } else {
> > >> if (fgets(pc->command_line, BUFSIZE-1, stdin) == NULL)
> > >> clean_exit(1);
> > >> + fprintf(fp, "%s%s", pc->prompt, pc->command_line);
> > >> clean_line(pc->command_line);
> > >> strcpy(pc->orig_line, pc->command_line);
> > >> }
> > >
> > > Yeah, maybe print this before the fgets with fflush()? similar to [1],
> > > or does the following make sense?
> > >
> > > diff --git a/cmdline.c b/cmdline.c
> > > index ded6551..181800a 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);
> >
> > It does not print the input command line only with this, I would like to
> > also print it if we print the prompt. How about this?
> >
>
> Right, this will only print the prompt but not the command line.
>
> I think either printing the command line or not are fine:
> - Not printing: In TTY mode, the command line won't be repeated again
> either. Also compared to gdb's behavior with the same use case, this
> is what gdb does.
> - Printing: In the `echo sys | crash` case, printing the command line
> gives more context.
>
> > --- 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,11 @@ 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);
>
> nit: Add a \n after %s?
>
> > + fflush(fp);
> > + }
> > clean_line(pc->command_line);
> > strcpy(pc->orig_line, pc->command_line);
> > }
> >
> > This looks consistent with the other types of crash session.
> >
> > And we can still get only a command output with -s option.
> >
> > # echo sys | crash -s
>
> Agreed.
>
> Thanks.
hi Kazu,
I sent a patch based on the discussion here. But please ignore it if
you already have a patch.
Thanks.
> >
> > Thanks,
> > Kazu
> >
> > >
> > >>
> > >> If you want the prompt and command with pre-generated crash commands,
> > >> you can use "crash -i" option. How about this?
> > >
> > > Filemode is not ideal if we want to pipe crash with another process
> > > that users interact with, since we will have to load vmlinux and
> > > dumpfile every time.
> > >
> > > But if this still looks like working as intended, I will close this issue.
> > >
> > > Thanks for your reply.
> > >
> > >>
> > >> # echo sys > cmd
> > >> # crash -i cmd
> > >>
> > >> Thanks,
> > >> Kazu
> > >>
> > >>>
> > >>> Thanks!
> > >>>
> > >>> [1] https://github.com/crash-utility/crash/blob/05a3a328fcd8920e49926b6d1c9c81ce0b6acbca/cmdline.c#L67
> > >>> [2] https://github.com/crash-utility/crash/blob/8246dce99dd23457e8c7a3fe9609c706694d1959/main.c#L1182
> > >>> [3] https://github.com/crash-utility/crash/blob/05a3a328fcd8920e49926b6d1c9c81ce0b6acbca/cmdline.c#L120
> > >>>
> > >>> --
> > >>> Crash-utility mailing list
> > >>> Crash-utility@xxxxxxxxxx
> > >>> https://listman.redhat.com/mailman/listinfo/crash-utility
> > >>> Contribution Guidelines: https://github.com/crash-utility/crash/wiki
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki
