Hi Dave,
Here I attach as a file for the patch. Thanks.
Kind regards,
Daniel
Hi Daniel,
As I mentioned before, the concept seems reasonable, which I thought
was to allow a user to prevent error() messages from being intermingled
with command output by redirecting them somewhere else. But that's
apparently not the case, as a few simple examples show otherwise.
Here's the default setting, and a sample command generating an error:
crash> set -v
scroll: on (/usr/bin/less)
radix: 10 (decimal)
refresh: on
print_max: 256
print_array: off
console: (not assigned)
debug: 0
core: off
hash: on
silent: off
edit: vi
namelist: /usr/lib/debug/lib/modules/3.10.0-957.21.2.el7.x86_64/vmlinux
dumpfile: (null)
unwind: off
zero_excluded: off
null-stop: off
gdb: off
scope: 0 (not set)
offline: show
redzone: on
stderr: stdout
crash> bt 33333
bt: invalid task or pid value: 33333
crash>
Here I set stderr to /dev/null, which sets the new pc->stderr,
but the behavior is still the same:
crash> set stderr /dev/null
stderr: /dev/null
crash> set -v
scroll: on (/usr/bin/less)
radix: 10 (decimal)
refresh: on
print_max: 256
print_array: off
console: (not assigned)
debug: 0
core: off
hash: on
silent: off
edit: vi
namelist: /usr/lib/debug/lib/modules/3.10.0-957.21.2.el7.x86_64/vmlinux
dumpfile: (null)
unwind: off
zero_excluded: off
null-stop: off
gdb: off
scope: 0 (not set)
offline: show
redzone: on
stderr: /dev/null
crash> bt 33333
bt: invalid task or pid value: 33333
crash>
Or if I set it to a file, the same thing happens:
crash> set stderr /tmp/junk
stderr: /tmp/junk
crash> set -v
scroll: on (/usr/bin/less)
radix: 10 (decimal)
refresh: on
print_max: 256
print_array: off
console: (not assigned)
debug: 0
core: off
hash: on
silent: off
edit: vi
namelist: /usr/lib/debug/lib/modules/3.10.0-957.21.2.el7.x86_64/vmlinux
dumpfile: (null)
unwind: off
zero_excluded: off
null-stop: off
gdb: off
scope: 0 (not set)
offline: show
redzone: on
stderr: /tmp/junk
crash> bt 33333
bt: invalid task or pid value: 33333
crash>
With your patch applied, the help page indicates:
stderr stdout | fp | <path> set the direction of error put. 'stdout'
always
print on console. 'fp' follows the redirection
or pipe command. '<path>' can be any file
path
in the filesystem which can save the output
Is states that "<path>" can be any file path in the filesystem which can
save
the output. But even I redirect a command, it still doesn't seem to do what
it states:
crash> set stderr /dev/null
stderr: /dev/null
crash> bt 33333 > /tmp/junk
crash> !cat /tmp/junk
bt: invalid task or pid value: 33333
crash>
Or if I pipe it:
crash> bt 33333 | cat
bt: invalid task or pid value: 33333
crash>
What am I missing?
And also, a couple more things...
Please make pc->stderr_path a pointer:
--- a/defs.h
+++ b/defs.h
@@ -553,6 +553,8 @@ struct program_context {
ulong scope; /* optional text context address */
ulong nr_hash_queues; /* hash queue head count */
char *(*read_vmcoreinfo)(const char *);
+ FILE *stderr; /* error() message direction */
+ char stderr_path[PATH_MAX]; /* stderr path information */
};
Since it's typically going to contain a handful of bytes, it's kind of wasteful
to use PATH_MAX (4096). Just use malloc/free to get a buffer of the correct
length.
And please display pc->stderr and pc->stderr_path to dump_program_context()
for use by "help -p".
Thanks,
Dave
On Sat, Jun 22, 2019 at 5:24 AM Dave Anderson < anderson(a)redhat.com > wrote:
Hi Daniel,
The idea seems reasonable, but the patch below is malformed:
$ patch -p1 < error.patch
checking file defs.h
Hunk #1 FAILED at 553.
1 out of 1 hunk FAILED
checking file help.c
patch: **** malformed patch at line 52: displayed by",
$
You can see that there are a quite a few unintended line wraps
in the patch below.
Can you make the patch a discrete attachment to your email?
Thanks,
Dave
----- Original Message -----
> Currently, the error() is always printing the output to the console
> through 'stdout'. This does not follow redirection which is good when
> you want to know error while redirecting commands output to a file.
> However, there are situations that you want to hide error messages or
> redirect it into somewhere else.
>
> Using 'set stderr' command, it can be changed to three different
> destination - fixed 'stdout', following redirection (fp), or a custom
> file path.
>
> crash> set stderr
> stderr: stdout
> crash> sym 0x523 > /dev/null
> sym: invalid address: 0x523
> crash> set stderr fp
> stderr: fp
> crash> sym 0x523 > /dev/null
> crash> set stderr /tmp/err.log
> stderr: /tmp/err.log
> crash> sym 0x523 > /dev/null
> crash> set stderr stdout
> stderr: stdout
> crash> sym 0x523 > /dev/null
> sym: invalid address: 0x523
> ---
> defs.h | 2 ++
> help.c | 5 +++++
> main.c | 2 ++
> tools.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
> 4 files changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/defs.h b/defs.h
> index ccffe58..57850c6 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -553,6 +553,8 @@ struct program_context {
> ulong scope; /* optional text context address */
> ulong nr_hash_queues; /* hash queue head count */
> char *(*read_vmcoreinfo)(const char *);
> + FILE *stderr; /* error() message direction */
> + char stderr_path[PATH_MAX]; /* stderr path information */
> };
>
> #define READMEM pc->readmem
> diff --git a/help.c b/help.c
> index 581e616..ddc8e86 100644
> --- a/help.c
> +++ b/help.c
> @@ -1093,6 +1093,10 @@ char *help_set[] = {
> " redzone on | off if on, CONFIG_SLUB object addresses
> displayed by",
> " the kmem command will point to the
> SLAB_RED_ZONE",
> " padding inserted at the beginning of
> the object.",
> +" stderr stdout | fp | <path> set the direction of error put.
> 'stdout' always",
> +" print on console. 'fp' follows the
> redirection",
> +" or pipe command. '<path>' can be any
> file path",
> +" in the filesystem which can save the
> output",
> " ",
> " Internal variables may be set in four manners:\n",
> " 1. entering the set command in $HOME/.%src.",
> @@ -1144,6 +1148,7 @@ char *help_set[] = {
> " scope: (not set)",
> " offline: show",
> " redzone: on",
> +" stderr: stdout",
> " ",
> " Show the current context:\n",
> " %s> set",
> diff --git a/main.c b/main.c
> index 83ccd31..68bdec4 100644
> --- a/main.c
> +++ b/main.c
> @@ -1085,6 +1085,8 @@ setup_environment(int argc, char **argv)
> * to pipes or output files.
> */
> fp = stdout;
> + pc->stderr = stdout;
> + strcpy(pc->stderr_path, "stdout");
>
> /*
> * Start populating the program_context structure. It's used so
> diff --git a/tools.c b/tools.c
> index 2d95c3a..840d07c 100644
> --- a/tools.c
> +++ b/tools.c
> @@ -58,6 +58,9 @@ __error(int type, char *fmt, ...)
> void *retaddr[NUMBER_STACKFRAMES] = { 0 };
> va_list ap;
>
> + if (!strcmp(pc->stderr_path, "fp"))
> + pc->stderr = fp;
> +
> if (CRASHDEBUG(1) || (pc->flags & DROP_CORE)) {
> SAVE_RETURN_ADDRESS(retaddr);
> console("error() trace: %lx => %lx => %lx => %lx\n",
> @@ -69,7 +72,7 @@ __error(int type, char *fmt, ...)
> va_end(ap);
>
> if (!fmt && FATAL_ERROR(type)) {
> - fprintf(stdout, "\n");
> + fprintf(pc->stderr, "\n");
> clean_exit(1);
> }
>
> @@ -95,14 +98,14 @@ __error(int type, char *fmt, ...)
> buf);
> fflush(pc->stdpipe);
> } else {
> - fprintf(stdout, "%s%s%s %s%s",
> + fprintf(pc->stderr, "%s%s%s %s%s",
> new_line || end_of_line ? "\n" : "",
> type == WARNING ? "WARNING" :
> type == NOTE ? "NOTE" :
> type == CONT ? spacebuf : pc->curcmd,
> type == CONT ? " " : ":",
> buf, end_of_line ? "\n" : "");
> - fflush(stdout);
> + fflush(pc->stderr);
> }
>
> if ((fp != stdout) && (fp != pc->stdpipe) && (fp !=
pc->tmpfile)) {
> @@ -2483,6 +2486,51 @@ cmd_set(void)
> }
> return;
>
> + } else if (STREQ(args[optind], "stderr")) {
> + if (args[optind+1]) {
> + FILE *tmp_fp = NULL;
> + char tmp_path[PATH_MAX];
> +
> + optind++;
> + if (STREQ(args[optind], "stdout")) {
> + tmp_fp = stdout;
> + strcpy(tmp_path, "stdout");
> + } else if (STREQ(args[optind], "fp")) {
> + tmp_fp = fp;
> + strcpy(tmp_path, "fp");
> + } else {
> + if (strlen(args[optind]) >=
> PATH_MAX) {
> + error(INFO, "path
> length %d is too long. (max=%d)\n",
> +
> strlen(args[optind]), PATH_MAX);
> + return;
> + }
> + tmp_fp = fopen(args[optind], "a");
> + if (tmp_fp != NULL) {
> + strcpy(tmp_path,
> args[optind]);
> + } else {
> + error(INFO, "invalid
> path: %s\n",
> + args[optind]);
> + return;
> + }
> +
> + }
> +
> + if (strcmp(pc->stderr_path, tmp_path)) {
> + if (strcmp(pc->stderr_path,
> "stdout")
> + && strcmp(pc->stderr_path,
> "fp")) {
> + fclose(pc->stderr);
> + }
> + pc->stderr = tmp_fp;
> + strcpy(pc->stderr_path, tmp_path);
> + }
> + }
> +
> + if (runtime) {
> + fprintf(fp, "stderr: %s\n",
> + pc->stderr_path);
> + }
> + return;
> +
> } else if (XEN_HYPER_MODE()) {
> error(FATAL, "invalid argument for the Xen hypervisor\n");
> } else if (pc->flags & MINIMAL_MODE) {
> @@ -2590,6 +2638,7 @@ show_options(void)
> fprintf(fp, "(not set)\n");
> fprintf(fp, " offline: %s\n", pc->flags2 & OFFLINE_HIDE ?
> "hide" : "show");
> fprintf(fp, " redzone: %s\n", pc->flags2 & REDZONE ? "on"
: "off");
> + fprintf(fp, " stderr: %s\n", pc->stderr_path);
> }
>
>
> --
> 1.8.3.1
>
> --
> Crash-utility mailing list
> Crash-utility(a)redhat.com
>
https://www.redhat.com/mailman/listinfo/crash-utility
>
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility