| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 14 Oct 2019 17:35:49 -0700
From: Ian Rogers <irogers@...gle.com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Jin Yao <yao.jin@...ux.intel.com>,
Song Liu <songliubraving@...com>,
LKML <linux-kernel@...r.kernel.org>,
clang-built-linux <clang-built-linux@...glegroups.com>,
Stephane Eranian <eranian@...gle.com>
Subject: Re: [PATCH 2/5] perf annotate: use run-command.h to fork objdump
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
<arnaldo.melo@...il.com> 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 <irogers@...gle.com>
> > ---
> > 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
Powered by blists - more mailing lists