Thanks for applying! Apologies, there was a missing null termination. Sent in a follow-up patch.
Ian On Mon, Oct 14, 2019 at 7:18 AM Arnaldo Carvalho de Melo <[email protected]> wrote: > > Em Thu, Oct 10, 2019 at 11:36:46AM -0700, Ian Rogers escreveu: > > Reduce duplicated logic by using the subcmd library. Ensure when errors > > occur they are reported to the caller. Before this patch, if no lines are > > read the error status is 0. > > Thanks, applied and tested. > > - Arnaldo > > > Signed-off-by: Ian Rogers <[email protected]> > > --- > > tools/perf/util/annotate.c | 71 +++++++++++++++++++------------------- > > 1 file changed, 36 insertions(+), 35 deletions(-) > > > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > > index 1487849a191a..fc12c5cfe112 100644 > > --- a/tools/perf/util/annotate.c > > +++ b/tools/perf/util/annotate.c > > @@ -43,6 +43,7 @@ > > #include <linux/string.h> > > #include <bpf/libbpf.h> > > #include <subcmd/parse-options.h> > > +#include <subcmd/run-command.h> > > > > /* FIXME: For the HE_COLORSET */ > > #include "ui/browser.h" > > @@ -1843,12 +1844,18 @@ static int symbol__disassemble(struct symbol *sym, > > struct annotate_args *args) > > struct kcore_extract kce; > > bool delete_extract = false; > > bool decomp = false; > > - int stdout_fd[2]; > > int lineno = 0; > > int nline; > > - pid_t pid; > > char *line; > > size_t line_len; > > + const char *objdump_argv[] = { > > + "/bin/sh", > > + "-c", > > + NULL, /* Will be the objdump command to run. */ > > + "--", > > + NULL, /* Will be the symfs path. */ > > + }; > > + struct child_process objdump_process; > > int err = dso__disassemble_filename(dso, symfs_filename, > > sizeof(symfs_filename)); > > > > if (err) > > @@ -1878,7 +1885,7 @@ static int symbol__disassemble(struct symbol *sym, > > struct annotate_args *args) > > > > if (dso__decompress_kmodule_path(dso, symfs_filename, > > tmp, sizeof(tmp)) < 0) > > - goto out; > > + return -1; > > > > decomp = true; > > strcpy(symfs_filename, tmp); > > @@ -1903,38 +1910,28 @@ static int symbol__disassemble(struct symbol *sym, > > struct annotate_args *args) > > > > pr_debug("Executing: %s\n", command); > > > > - err = -1; > > - if (pipe(stdout_fd) < 0) { > > - pr_err("Failure creating the pipe to run %s\n", command); > > - goto out_free_command; > > - } > > - > > - pid = fork(); > > - if (pid < 0) { > > - pr_err("Failure forking to run %s\n", command); > > - goto out_close_stdout; > > - } > > + objdump_argv[2] = command; > > + objdump_argv[4] = symfs_filename; > > > > - if (pid == 0) { > > - close(stdout_fd[0]); > > - dup2(stdout_fd[1], 1); > > - close(stdout_fd[1]); > > - execl("/bin/sh", "sh", "-c", command, "--", symfs_filename, > > - NULL); > > - perror(command); > > - exit(-1); > > + /* Create a pipe to read from for stdout */ > > + memset(&objdump_process, 0, sizeof(objdump_process)); > > + objdump_process.argv = objdump_argv; > > + objdump_process.out = -1; > > + if (start_command(&objdump_process)) { > > + pr_err("Failure starting to run %s\n", command); > > + err = -1; > > + goto out_free_command; > > } > > > > - close(stdout_fd[1]); > > - > > - file = fdopen(stdout_fd[0], "r"); > > + file = fdopen(objdump_process.out, "r"); > > if (!file) { > > pr_err("Failure creating FILE stream for %s\n", command); > > /* > > * If we were using debug info should retry with > > * original binary. > > */ > > - goto out_free_command; > > + err = -1; > > + goto out_close_stdout; > > } > > > > /* Storage for getline. */ > > @@ -1958,8 +1955,14 @@ static int symbol__disassemble(struct symbol *sym, > > struct annotate_args *args) > > } > > free(line); > > > > - if (nline == 0) > > + err = finish_command(&objdump_process); > > + if (err) > > + pr_err("Error running %s\n", command); > > + > > + if (nline == 0) { > > + err = -1; > > pr_err("No output from %s\n", command); > > + } > > > > /* > > * kallsyms does not have symbol sizes so there may a nop at the end. > > @@ -1969,23 +1972,21 @@ static int symbol__disassemble(struct symbol *sym, > > struct annotate_args *args) > > delete_last_nop(sym); > > > > fclose(file); > > - err = 0; > > + > > +out_close_stdout: > > + close(objdump_process.out); > > + > > out_free_command: > > free(command); > > -out_remove_tmp: > > - close(stdout_fd[0]); > > > > +out_remove_tmp: > > if (decomp) > > unlink(symfs_filename); > > > > if (delete_extract) > > kcore_extract__delete(&kce); > > -out: > > - return err; > > > > -out_close_stdout: > > - close(stdout_fd[1]); > > - goto out_free_command; > > + return err; > > } > > > > static void calc_percent(struct sym_hist *sym_hist, > > -- > > 2.23.0.581.g78d2f28ef7-goog > > -- > > - Arnaldo
