Re: [Crash-utility] [PATCH] trace: Improve "trace show" command

----- Original Message ----- > On 03/03/2011 11:25 PM, Dave Anderson wrote: >> >> >> ----- Original Message ----- >>> The code were also applied to: >>> git://github.com/laijs/tracing-extension-module-for-crash.git >>> >>> Documents and man pages will/may be added in two weeks. >>> >>> Dave Anderson, could you add a "Requires" entry to its RPM.spec, >>> it requires trace-cmd RPM after this patch applied. > > Sorry, please omit this requirement of mine. That actually would be a legitimate request for the RHEL6 crash-trace-command package, but I'd prefer not to add a Requires entry for the upstream src.rpm. But I note in your new patch, that ftrace_show() is a void function, and if the popen("trace.cmd") fails, it just returns quietly. I would think that there should be an explanatory error message in that case? Also, there's a buffer overflow error assignment to buf[4097]: static void ftrace_show(int argc, char *argv[]) { char buf[4096]; char tmp[] = "/tmp/crash.trace_dat.XXXXXX"; char *trace_cmd = "trace-cmd", *env_trace_cmd = getenv("TRACE_CMD"); int fd; FILE *file; size_t ret; /* check trace-cmd */ if (env_trace_cmd) trace_cmd = env_trace_cmd; if (!(file = popen(trace_cmd, "r"))) return; ret = fread(buf, 1, sizeof(buf), file); buf[4097] = 0; if (!strstr(buf, "trace-cmd version")) { if (env_trace_cmd) fprintf(fp, "Invalid environment TRACE_CMD: %s\n", env_trace_cmd); else fprintf(fp, "\"trace show\" requires trace-cmd.\n" "please set the environment TRACE_CMD " "if you installed it a special path\n" ); return; } So if you don't mind, I'll fix it like this: buf[0] = 0; if ((file = popen(trace_cmd, "r"))) { ret = fread(buf, 1, sizeof(buf), file); buf[4095] = 0; }

if (!strstr(buf, "trace-cmd version")) { if (env_trace_cmd) fprintf(fp, "Invalid environment TRACE_CMD: %s\n", env_trace_cmd); else fprintf(fp, "\"trace show\" requires trace-cmd.\n" "please set the environment TRACE_CMD " "if you installed it in a special path\n" ); return; } If that's OK with you, the patch is queued for crash 5.1.4.