mbox series

[v1,0/7] Add and use run_command_strbuf

Message ID 20230110222003.1591436-1-irogers@google.com
Headers
Series Add and use run_command_strbuf |

Message

Ian Rogers Jan. 10, 2023, 10:19 p.m. UTC 
  It is commonly useful to run a command using "/bin/sh -c" (like popen)
and to place the output in a string. Move strbuf to libapi, add a new
run_command that places output in a strbuf, then use it in help and
llvm in perf. Some small strbuf efficiency improvements are
included. Whilst adding a new function should increase lines-of-code,
by sharing two similar usages in perf llvm and perf help, the overall
lines-of-code is moderately reduced.

First "perf llvm: Fix inadvertent file creation" is cherry-picked
from:
https://lore.kernel.org/lkml/20230105082609.344538-1-irogers@google.com/
to avoid a merge conflict. The next patches deal with moving strbuf,
adding the run_command function with Makefile dependency from
libsubcmd to libapi, and improving the strbuf performance. The final
two patches add usage from the perf command.

Ian Rogers (7):
  perf llvm: Fix inadvertent file creation
  tools lib: Move strbuf to libapi
  tools lib subcmd: Add run_command_strbuf
  tools lib api: Minor strbuf_read improvements
  tools lib api: Tweak strbuf allocation size computation
  perf help: Use run_command_strbuf
  perf llvm: Remove read_from_pipe

 tools/lib/api/Build                   |   1 +
 tools/lib/api/Makefile                |   2 +-
 tools/{perf/util => lib/api}/strbuf.c |  28 ++--
 tools/{perf/util => lib/api}/strbuf.h |   0
 tools/lib/subcmd/Makefile             |  32 +++-
 tools/lib/subcmd/run-command.c        |  30 ++++
 tools/lib/subcmd/run-command.h        |  14 ++
 tools/perf/bench/evlist-open-close.c  |   2 +-
 tools/perf/builtin-help.c             |  49 ++----
 tools/perf/builtin-list.c             |   2 +-
 tools/perf/tests/bpf.c                |  12 +-
 tools/perf/tests/llvm.c               |  18 +--
 tools/perf/tests/llvm.h               |   3 +-
 tools/perf/util/Build                 |   1 -
 tools/perf/util/bpf-loader.c          |   9 +-
 tools/perf/util/cache.h               |   2 +-
 tools/perf/util/dwarf-aux.c           |   2 +-
 tools/perf/util/env.c                 |   2 +-
 tools/perf/util/header.c              |   2 +-
 tools/perf/util/llvm-utils.c          | 207 ++++++++------------------
 tools/perf/util/llvm-utils.h          |   6 +-
 tools/perf/util/metricgroup.c         |   2 +-
 tools/perf/util/pfm.c                 |   2 +-
 tools/perf/util/pmu.c                 |   2 +-
 tools/perf/util/probe-event.c         |   2 +-
 tools/perf/util/probe-file.c          |   2 +-
 tools/perf/util/probe-finder.c        |   2 +-
 tools/perf/util/sort.c                |   2 +-
 28 files changed, 201 insertions(+), 237 deletions(-)
 rename tools/{perf/util => lib/api}/strbuf.c (87%)
 rename tools/{perf/util => lib/api}/strbuf.h (100%)

Comments

Ian Rogers Jan. 19, 2023, 4:05 p.m. UTC | #1 
On Tue, Jan 10, 2023 at 2:20 PM Ian Rogers <irogers@google.com> wrote:
>
> It is commonly useful to run a command using "/bin/sh -c" (like popen)
> and to place the output in a string. Move strbuf to libapi, add a new
> run_command that places output in a strbuf, then use it in help and
> llvm in perf. Some small strbuf efficiency improvements are
> included. Whilst adding a new function should increase lines-of-code,
> by sharing two similar usages in perf llvm and perf help, the overall
> lines-of-code is moderately reduced.
>
> First "perf llvm: Fix inadvertent file creation" is cherry-picked
> from:
> https://lore.kernel.org/lkml/20230105082609.344538-1-irogers@google.com/
> to avoid a merge conflict. The next patches deal with moving strbuf,
> adding the run_command function with Makefile dependency from
> libsubcmd to libapi, and improving the strbuf performance. The final
> two patches add usage from the perf command.
>
> Ian Rogers (7):
>   perf llvm: Fix inadvertent file creation
>   tools lib: Move strbuf to libapi
>   tools lib subcmd: Add run_command_strbuf
>   tools lib api: Minor strbuf_read improvements
>   tools lib api: Tweak strbuf allocation size computation
>   perf help: Use run_command_strbuf
>   perf llvm: Remove read_from_pipe

This isn't ready yet. Kernel test robot reported legitimate build
breakages in other tools outside of perf, I'm looking to address those
in separate patch series.
https://lore.kernel.org/lkml/20230116215751.633675-1-irogers@google.com/

Thanks,
Ian

>  tools/lib/api/Build                   |   1 +
>  tools/lib/api/Makefile                |   2 +-
>  tools/{perf/util => lib/api}/strbuf.c |  28 ++--
>  tools/{perf/util => lib/api}/strbuf.h |   0
>  tools/lib/subcmd/Makefile             |  32 +++-
>  tools/lib/subcmd/run-command.c        |  30 ++++
>  tools/lib/subcmd/run-command.h        |  14 ++
>  tools/perf/bench/evlist-open-close.c  |   2 +-
>  tools/perf/builtin-help.c             |  49 ++----
>  tools/perf/builtin-list.c             |   2 +-
>  tools/perf/tests/bpf.c                |  12 +-
>  tools/perf/tests/llvm.c               |  18 +--
>  tools/perf/tests/llvm.h               |   3 +-
>  tools/perf/util/Build                 |   1 -
>  tools/perf/util/bpf-loader.c          |   9 +-
>  tools/perf/util/cache.h               |   2 +-
>  tools/perf/util/dwarf-aux.c           |   2 +-
>  tools/perf/util/env.c                 |   2 +-
>  tools/perf/util/header.c              |   2 +-
>  tools/perf/util/llvm-utils.c          | 207 ++++++++------------------
>  tools/perf/util/llvm-utils.h          |   6 +-
>  tools/perf/util/metricgroup.c         |   2 +-
>  tools/perf/util/pfm.c                 |   2 +-
>  tools/perf/util/pmu.c                 |   2 +-
>  tools/perf/util/probe-event.c         |   2 +-
>  tools/perf/util/probe-file.c          |   2 +-
>  tools/perf/util/probe-finder.c        |   2 +-
>  tools/perf/util/sort.c                |   2 +-
>  28 files changed, 201 insertions(+), 237 deletions(-)
>  rename tools/{perf/util => lib/api}/strbuf.c (87%)
>  rename tools/{perf/util => lib/api}/strbuf.h (100%)
>
> --
> 2.39.0.314.g84b9a713c41-goog
>
Arnaldo Carvalho de Melo Jan. 19, 2023, 4:28 p.m. UTC | #2 
Em Thu, Jan 19, 2023 at 08:05:36AM -0800, Ian Rogers escreveu:
> On Tue, Jan 10, 2023 at 2:20 PM Ian Rogers <irogers@google.com> wrote:
> >
> > It is commonly useful to run a command using "/bin/sh -c" (like popen)
> > and to place the output in a string. Move strbuf to libapi, add a new
> > run_command that places output in a strbuf, then use it in help and
> > llvm in perf. Some small strbuf efficiency improvements are
> > included. Whilst adding a new function should increase lines-of-code,
> > by sharing two similar usages in perf llvm and perf help, the overall
> > lines-of-code is moderately reduced.
> >
> > First "perf llvm: Fix inadvertent file creation" is cherry-picked
> > from:
> > https://lore.kernel.org/lkml/20230105082609.344538-1-irogers@google.com/
> > to avoid a merge conflict. The next patches deal with moving strbuf,
> > adding the run_command function with Makefile dependency from
> > libsubcmd to libapi, and improving the strbuf performance. The final
> > two patches add usage from the perf command.
> >
> > Ian Rogers (7):
> >   perf llvm: Fix inadvertent file creation
> >   tools lib: Move strbuf to libapi
> >   tools lib subcmd: Add run_command_strbuf
> >   tools lib api: Minor strbuf_read improvements
> >   tools lib api: Tweak strbuf allocation size computation
> >   perf help: Use run_command_strbuf
> >   perf llvm: Remove read_from_pipe
> 
> This isn't ready yet. Kernel test robot reported legitimate build
> breakages in other tools outside of perf, I'm looking to address those
> in separate patch series.
> https://lore.kernel.org/lkml/20230116215751.633675-1-irogers@google.com/

Thanks for the heads up, I recall seeing a build bot report.

It's great to have that build bot testing perf patches, thanks to
whoever put it in place!

- Arnaldo