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
