Hi Daniel, This patch looks good. I did make a few trivial changes: In set_error(), The NULL check is unnecessary when using STREQ(): + if (pc->error_path != NULL && STREQ(target, pc->error_path)) + return pc->error_fp; I also tried to clarify the help page description, made the set_error() call from setup_environment() check its return value and exit gracefully in the extremely unlikely event of a malloc() failure, and cleaned up the whitespace usage throughout the patch. Queued for crash-7.2.7: https://github.com/crash-utility/crash/commit/e6d8f0d8c78b2dbaa13765e9e7da456d0e64a268 Thanks, Dave ----- Original Message ----- > Hi Dave, > > There was a missed corner case that it doesn't work if scroll is on. The > new patch is passed both scroll on and off. > > First, this doesn't work as you've advertised: > > > > crash> set stderr /dev/null > > stderr: /dev/null > > crash> sym 555 > > sym: invalid address: 555 > > crash> > > > The new patch applied you had mentioned. I was also unhappy with name 'fp'. > > > > > > Secondly, could you change the name of the "stderr" variable? It's > > kind of confusing, especially since the crash internals specifically > > avoid using stderr. Perhaps use "error" instead? And then with respect > > to the 3 possible options, "fp" is confusing and somewhat meaningless > > to a user -- let's use "redirect", since that's what you're describing. > > > > Internally, please clarify/rename the "pc->stderr" and "pc->stderr_path" > > member > > names to be "pc->error_fp" and "pc->error_path". And in > > dump_program_context(), > > display both members explicitly as such, not the way you are displaying > > the stderr FILE pointer as the stderr_path string: > > > > + fprintf(fp, " stderr: %s\n", pc->stderr_path); > > > > > In the case of 'redirect', we have to update file pointer on the fly as it > can have different value from the default stdout or current fp. > > > At the beginning of __error(), you've got this: > > > > + if (!strcmp(pc->stderr_path, "fp")) > > + pc->stderr = fp; > > + > > > > I don't understand that. What's going on there? > > > > > Applied STREQ() for those locations. > > Kind regards, > Daniel Kwon > > > And lastly, just for my own sanity, in all the places where you're > > using strcmp() and !strcmp(), can you please just use STREQ() instead? > > > > Thanks, > > Dave > > > > > > > > > > > > ----- Original Message ----- > > > I shouldn't use that message in the previous email. Yes, I am writing > > > a command that does extra actions while doing 'dis' which gives you > > > some extra information to make it easy to understand the code. As a > > > part of it, it is trying to find symbols for any given values. > > > > > > Anyway, here I put another example that just does calling 'sym' > > > command in exec_crash_command(). You can try it once load 'mpykdump' > > > shared object. > > > > > > > > ----------------------------------------------------------------------------------- > > > $ pwd > > > /root/Work > > > $ cat test_cmd.py > > > from pykdump.API import * > > > > > > def test_cmd(): > > > try: > > > sym = exec_crash_command("sym 0x123") > > > print("{%s}" % sym) > > > except: > > > pass > > > > > > > > > if ( __name__ == '__main__'): > > > test_cmd() > > > > > > $ ./crash > > > ... > > > crash> extend /root/.crash.d/mpykdump.so > > > /root/.crash.d/mpykdump.so: shared object loaded > > > > > > crash> set stderr default > > > stderr: default > > > crash> epython /root/Work/test_cmd.py > > > sym: invalid address: 0x123 > > > {sym: invalid address: 0x123 > > > } > > > > > > ** Execution took 0.00s (real) 0.00s (CPU) > > > crash> set stderr fp > > > stderr: fp > > > crash> epython /root/Work/test_cmd.py > > > {sym: invalid address: 0x123 > > > } > > > > > > ** Execution took 0.00s (real) 0.00s (CPU) > > > crash> set stderr /tmp/output > > > stderr: /tmp/output > > > crash> epython /root/Work/test_cmd.py > > > crash> !cat /tmp/output > > > sym: invalid address: 0x123 > > > > > ----------------------------------------------------------------------------------- > > > > > > Yes, in the above by using 'fp', we can redirect the error message > > > into the same exec_crash_command() and prevent showing any messages on > > > console. By doing that, we can have a full control for the messages. > > > If we want to seperate error messages from the beginning, but not > > > making duplicate, we could use file path instead. > > > > > > PS. I am attaching a new patch that has a small change as there was a > > > memory leak when file open is failed. > > > > > > Regards, > > > Daniel Kwon > > > > > > On Sun, Jun 30, 2019 at 1:42 AM Dave Anderson <anderson@xxxxxxxxxx> > > wrote: > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > I am writing an extension based on 'mpykdump' and calling > > > > > 'exec_crash_command()' to find symbol details as shown below. > > > > > > > > > > --- > > > > > def find_symbol(str): > > > > > try: > > > > > sym = exec_crash_command("sym %s" % str) > > > > > if sym.startswith("sym:") != True: > > > > > return " <" + "".join(sym.split()[2:]) + ">" > > > > > except: > > > > > pass > > > > > > > > > > return "" > > > > > --- > > > > > > > > > > In the below example, I am trying to interpret address > > > > > '0xffff92a3fba0f100' by calling 'sym' crash command, but can't handle > > > > > the situation where the symbol doesn't exist. > > > > > > > > > > --- > > > > > ... > > > > > 0xffffffff92858b75 <do_sys_poll+741>: callq 0xffffffff92986f80 > > > > > <__x86_indirect_thunk_rdx> > > > > > sym: invalid address: 0xffff92a3fba0f100 > > > > > 0xffffffff92858b7a <do_sys_poll+746>: mov 0x50(%rsp),%rcx > > > > > ;0xffff92a3fba0f100 > > > > > ... > > > > > --- > > > > > > > > > > By using 'fp', I can redirect the error into the > > > > > 'exec_crash_command()' result which I can use to identify the reason > > > > > if there's error and can avoid showing unnecessary error on console. > > > > > > > > If you'll allow me to continue beating a dead horse... > > > > > > > > I've never used mpykdump before, and it's confusing because you are > > showing > > > > disassembly > > > > output, with a "sym" command error message appearing in the middle. > > Are > > > > you calling > > > > excec_crash_command() while you are parsing disassembly output from > > within > > > > some other > > > > mypkdump execution stream? > > > > > > > > Anyway, are you saying that -- with the current behavior -- when you > > > > redirect the exec_crash_command() > > > > function, that the sym error messages do *not* go to the redirected > > file or > > > > pipe stream? > > > > > > > > It sounds like you're saying that your proposed "fp" setting would > > simply > > > > not display the extra > > > > "unnecessary" error message on the console. > > > > > > > > Dave > > > > > > > > > > > > > > > > > > Regards, > > > > > Daniel > > > > > > > > > > Kind regards, > > > > > > > > > > Daniel Kwon, RHCA > > > > > > > > > > Principle Software Maintenance Engineer, CEE > > > > > > > > > > Red Hat APAC > > > > > > > > > > > > > > > > > > > > On Fri, Jun 28, 2019 at 11:41 PM Dave Anderson <anderson@xxxxxxxxxx> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > Hi Dave, > > > > > > > > > > > > > > Yes, that is violating the default behaviour. I recheck how it > > should > > > > > > > be handled and made the below rules. > > > > > > > > > > > > > > - 'default' : Working just like the 'crash' before this 'stderr' > > > > > > > implementation. > > > > > > > - 'fp' : Only goes into one destination. It can be console in > > normal > > > > > > > command, but will go into target file if redirection or pipe is > > used. > > > > > > > > > > > > I still don't understand why you want to bother with this "fp" > > option? > > > > > > What's the problem you're trying to address? > > > > > > > > > > > > Dave > > > > > > > > > > > > > > > > > > > - '<path>' : It will go into the specified file only and no > > console > > > > > > > output. > > > > > > > > > > > > > > Below is the test I have done for the test. Hope this behaviour > > is > > > > > > > reasonable. > > > > > > > > > > > > > > --------------------------- > > > > > > > ## default: standard error handling behaviour. > > > > > > > > > > > > > > normal command: error prints on console > > > > > > > > > > > > > > crash> set stderr default > > > > > > > stderr: default > > > > > > > > > > > > > > crash> sym 0x123 > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > redirect: goes into both console and redirected file. > > > > > > > > > > > > > > crash> sym 0x123 > /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > crash> !cat /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > > > > > > > > pipe: goes into both console and piped direction. > > > > > > > > > > > > > > crash> sym 0x123 | cat > > > > > > > sym: invalid address: 0x123 > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > crash> sym 0x123 | cat > /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > crash> !cat /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ## fp: Only to the one target such as stdout, pipe, or redirected > > > > > > > file > > > > > > > > > > > > > > normal command: error prints on console > > > > > > > > > > > > > > crash> set stderr fp > > > > > > > stderr: fp > > > > > > > > > > > > > > crash> sym 0x123 > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > redirect: goes into redirected file only. > > > > > > > > > > > > > > crash> sym 0x123 > /tmp/output > > > > > > > crash> !cat /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > > > > > > > > pipe: goes into piped direction only. > > > > > > > > > > > > > > crash> sym 0x123 | cat > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > crash> sym 0x123 | cat > /tmp/output > > > > > > > > > > > > > > crash> !cat /tmp/output > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > > > > > > > > > > > > > > > ## <file path>: Only to the specified file. > > > > > > > > > > > > > > normal command: error goes into the specified file only. > > > > > > > > > > > > > > crash> set stderr /tmp/stderr > > > > > > > stderr: /tmp/stderr > > > > > > > crash> sym 0x123 > > > > > > > crash> !cat /tmp/stderr > > > > > > > sym: invalid address: 0x123 > > > > > > > > > > > > > > redirect: error goes into the specified file only. > > > > > > > > > > > > > > crash> sym 0x124 > /tmp/output > > > > > > > crash> !cat /tmp/output > > > > > > > crash> !cat /tmp/stderr > > > > > > > sym: invalid address: 0x123 > > > > > > > sym: invalid address: 0x124 > > > > > > > > > > > > > > pipe: error goes into the specified file only. > > > > > > > > > > > > > > crash> sym 0x125 | cat > > > > > > > crash> sym 0x126 | cat > /tmp/output > > > > > > > crash> !cat /tmp/output > > > > > > > crash> !cat /tmp/stderr > > > > > > > sym: invalid address: 0x123 > > > > > > > sym: invalid address: 0x124 > > > > > > > sym: invalid address: 0x125 > > > > > > > sym: invalid address: 0x126 > > > > > > > --------------------------- > > > > > > > > > > > > > > Regards, > > > > > > > Daniel Kwon > > > > > > > > > > > > > > Kind regards, > > > > > > > > > > > > > > Daniel Kwon, RHCA > > > > > > > > > > > > > > Principle Software Maintenance Engineer, CEE > > > > > > > > > > > > > > Red Hat APAC > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Jun 28, 2019 at 5:27 AM Dave Anderson < > > anderson@xxxxxxxxxx> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > > > Hi Dave, > > > > > > > > > > > > > > > > > > It looks like __error() function has an extra output which > > can > > > > > > > > > cause > > > > > > > > > of > > > > > > > > > confusion. I rewrote the code to cover that as well as the > > > > > > > > > changes > > > > > > > > > you > > > > > > > > > had > > > > > > > > > asked. Please let me know how it goes. > > > > > > > > > > > > > > > > Hi Daniel, > > > > > > > > > > > > > > > > Upon initial testing, I note that your patch changes the > > default > > > > > > > > behavior, > > > > > > > > which is unacceptable. > > > > > > > > > > > > > > > > By default, the idea is to get all error() messages out so that > > > > > > > > they > > > > > > > > are > > > > > > > > seen by the user regardless of how the command's output may be > > > > > > > > piped or > > > > > > > > redirected. So if a command's output is redirected to a file > > or > > > > > > > > pipe, > > > > > > > > the error message goes both to the console as well as being > > > > > > > > intermingled > > > > > > > > in the pipe/file command output. > > > > > > > > > > > > > > > > Taking your simple example, by default, command output and > > error > > > > > > > > messages > > > > > > > > are piped (behind the scenes) to /usr/bin/less: > > > > > > > > > > > > > > > > crash> sym 0x523 > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > If the default piping is turned off, command output and error > > > > > > > > messages > > > > > > > > go to stdout: > > > > > > > > > > > > > > > > crash> set scroll off > > > > > > > > scroll: off (/usr/bin/less) > > > > > > > > crash> sym 0x523 > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > However, if the command is redirected to a file, any command > > output > > > > > > > > and > > > > > > > > error > > > > > > > > messages go to the file, but error messages also go to the > > console: > > > > > > > > > > > > > > > > crash> sym 0x523 > /tmp/output > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> !cat /tmp/output > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > Similarly, if the command is piped to a command, command > > output and > > > > > > > > error > > > > > > > > messages > > > > > > > > go to the pipe, and error messages also go to the console: > > > > > > > > > > > > > > > > crash> sym 0x523 | cat > > > > > > > > sym: invalid address: 0x523 > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > So with your patch applied, and the new stderr variable set to > > the > > > > > > > > default > > > > > > > > of "stdout": > > > > > > > > > > > > > > > > crash> set stderr > > > > > > > > stderr: stdout > > > > > > > > crash> > > > > > > > > > > > > > > > > Let's run the same set of commands as above: > > > > > > > > > > > > > > > > crash> sym 0x523 > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> set scroll off > > > > > > > > scroll: off (/usr/bin/less) > > > > > > > > crash> sym 0x523 > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > Same behavior as always. However, if a command is redirected > > to a > > > > > > > > file, > > > > > > > > the error message only goes to the console, but it is not sent > > to > > > > > > > > the > > > > > > > > output file: > > > > > > > > > > > > > > > > crash> sym 0x523 > /tmp/output > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> !cat /tmp/output > > > > > > > > crash> > > > > > > > > > > > > > > > > Similarly, when piped to a command, the error message is only > > going > > > > > > > > to > > > > > > > > one of the destinations: > > > > > > > > > > > > > > > > crash> sym 0x523 | cat > > > > > > > > sym: invalid address: 0x523 > > > > > > > > crash> > > > > > > > > > > > > > > > > So there's no way I'm going to change behavior from what it has > > > > > > > > been forever. > > > > > > > > > > > > > > > > While I didn't test your alternative settings, it's not > > entirely > > > > > > > > clear > > > > > > > > what you're trying to accomplish. Seemingly it would make > > sense to > > > > > > > > have > > > > > > > > a binary setting for the new "stderr": > > > > > > > > > > > > > > > > (1) the current default behavior, or > > > > > > > > (2) a setting allowing you to redirect all error() messages > > to a > > > > > > > > designated file. > > > > > > > > > > > > > > > > Option (2) would *not* send them to the console *or* > > intermingle > > > > > > > > them > > > > > > > > with command output. But that's just me... > > > > > > > > > > > > > > > > Also, here's a minor compiler complaint: > > > > > > > > > > > > > > > > $ make warn > > > > > > > > ... > > > > > > > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 main.c -Wall -O2 > > > > > > > > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector > > > > > > > > -Wformat-security > > > > > > > > main.c: In function ‘setup_environment’: > > > > > > > > main.c:1088:9: warning: implicit declaration of function > > > > > > > > ‘set_stderr’ > > > > > > > > [-Wimplicit-function-declaration] > > > > > > > > set_stderr("stdout"); > > > > > > > > ^ > > > > > > > > cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_7_6 tools.c -Wall -O2 > > > > > > > > -Wstrict-prototypes -Wmissing-prototypes -fstack-protector > > > > > > > > -Wformat-security > > > > > > > > tools.c:42:1: warning: no previous prototype for ‘set_stderr’ > > > > > > > > [-Wmissing-prototypes] > > > > > > > > set_stderr(char *target) > > > > > > > > ^ > > > > > > > > ... > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Dave > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Kind regards, > > > > > > > > > Daniel Kwon > > > > > > > > > > > > > > > > > > On Tue, Jun 25, 2019 at 2:06 AM Dave Anderson > > > > > > > > > <anderson@xxxxxxxxxx> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > > > > > 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@xxxxxxxxxx > > > > > > > > > > > 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@xxxxxxxxxx > > > > > > > > > > > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > Crash-utility mailing list > > > > > > > > > > > Crash-utility@xxxxxxxxxx > > > > > > > > > > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > > > > > > Crash-utility mailing list > > > > > > > > > > > Crash-utility@xxxxxxxxxx > > > > > > > > > > > https://www.redhat.com/mailman/listinfo/crash-utility > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
