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.
>
> 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
