Switch to the common run_command_strbuf utility. Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> --- tools/perf/tests/bpf.c | 12 +-- tools/perf/tests/llvm.c | 18 ++-- tools/perf/tests/llvm.h | 3 +- tools/perf/util/bpf-loader.c | 9 +- tools/perf/util/llvm-utils.c | 184 +++++++---------------------------- tools/perf/util/llvm-utils.h | 6 +- 6 files changed, 61 insertions(+), 171 deletions(-) diff --git a/tools/perf/tests/bpf.c b/tools/perf/tests/bpf.c index 17c023823713..b4a6afb41e40 100644 --- a/tools/perf/tests/bpf.c +++ b/tools/perf/tests/bpf.c @@ -13,6 +13,7 @@ #include <linux/filter.h> #include <linux/kernel.h> #include <linux/string.h> +#include <api/strbuf.h> #include <api/fs/fs.h> #include <perf/mmap.h> #include "tests.h" @@ -216,14 +217,13 @@ prepare_bpf(void *obj_buf, size_t obj_buf_sz, const char *name) static int __test__bpf(int idx) { int ret; - void *obj_buf; - size_t obj_buf_sz; + struct strbuf obj_buf = STRBUF_INIT; struct bpf_object *obj; - ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz, + ret = test_llvm__fetch_bpf_obj(&obj_buf, bpf_testcase_table[idx].prog_id, false, NULL); - if (ret != TEST_OK || !obj_buf || !obj_buf_sz) { + if (ret != TEST_OK || !obj_buf.len) { pr_debug("Unable to get BPF object, %s\n", bpf_testcase_table[idx].msg_compile_fail); if ((idx == 0) || (ret == TEST_SKIP)) @@ -232,7 +232,7 @@ static int __test__bpf(int idx) return TEST_FAIL; } - obj = prepare_bpf(obj_buf, obj_buf_sz, + obj = prepare_bpf(obj_buf.buf, obj_buf.len, bpf_testcase_table[idx].name); if ((!!bpf_testcase_table[idx].target_func) != (!!obj)) { if (!obj) @@ -274,7 +274,7 @@ static int __test__bpf(int idx) } out: - free(obj_buf); + strbuf_release(&obj_buf); bpf__clear(); return ret; } diff --git a/tools/perf/tests/llvm.c b/tools/perf/tests/llvm.c index 0bc25a56cfef..97090850b7f9 100644 --- a/tools/perf/tests/llvm.c +++ b/tools/perf/tests/llvm.c @@ -4,6 +4,7 @@ #include <string.h> #include "tests.h" #include "debug.h" +#include <api/strbuf.h> #ifdef HAVE_LIBBPF_SUPPORT #include <bpf/libbpf.h> @@ -45,8 +46,7 @@ static struct { }; int -test_llvm__fetch_bpf_obj(void **p_obj_buf, - size_t *p_obj_buf_sz, +test_llvm__fetch_bpf_obj(struct strbuf *obj_buf, enum test_llvm__testcase idx, bool force, bool *should_load_fail) @@ -83,9 +83,6 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf, if (verbose == 0) verbose = -1; - *p_obj_buf = NULL; - *p_obj_buf_sz = 0; - if (!llvm_param.clang_bpf_cmd_template) goto out; @@ -106,7 +103,7 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf, clang_opt_old = llvm_param.clang_opt; llvm_param.clang_opt = clang_opt_new; - err = llvm__compile_bpf("-", p_obj_buf, p_obj_buf_sz); + err = llvm__compile_bpf("-", obj_buf); llvm_param.clang_bpf_cmd_template = tmpl_old; llvm_param.clang_opt = clang_opt_old; @@ -127,24 +124,23 @@ test_llvm__fetch_bpf_obj(void **p_obj_buf, static int test__llvm(int subtest) { int ret; - void *obj_buf = NULL; - size_t obj_buf_sz = 0; + struct strbuf obj_buf = STRBUF_INIT; bool should_load_fail = false; if ((subtest < 0) || (subtest >= __LLVM_TESTCASE_MAX)) return TEST_FAIL; - ret = test_llvm__fetch_bpf_obj(&obj_buf, &obj_buf_sz, + ret = test_llvm__fetch_bpf_obj(&obj_buf, subtest, false, &should_load_fail); if (ret == TEST_OK && !should_load_fail) { - ret = test__bpf_parsing(obj_buf, obj_buf_sz); + ret = test__bpf_parsing(obj_buf.buf, obj_buf.len); if (ret != TEST_OK) { pr_debug("Failed to parse test case '%s'\n", bpf_source_table[subtest].desc); } } - free(obj_buf); + strbuf_release(&obj_buf); return ret; } diff --git a/tools/perf/tests/llvm.h b/tools/perf/tests/llvm.h index f68b0d9b8ae2..2294b66dd0b6 100644 --- a/tools/perf/tests/llvm.h +++ b/tools/perf/tests/llvm.h @@ -22,7 +22,8 @@ enum test_llvm__testcase { __LLVM_TESTCASE_MAX, }; -int test_llvm__fetch_bpf_obj(void **p_obj_buf, size_t *p_obj_buf_sz, +struct strbuf; +int test_llvm__fetch_bpf_obj(struct strbuf *obj_buf, enum test_llvm__testcase index, bool force, bool *should_load_fail); #ifdef __cplusplus diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index 6e9b06cf06ee..f3a5cf490141 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -16,6 +16,7 @@ #include <linux/zalloc.h> #include <errno.h> #include <stdlib.h> +#include <api/strbuf.h> #include "debug.h" #include "evlist.h" #include "bpf-loader.h" @@ -245,10 +246,14 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source) err = perf_clang__compile_bpf(filename, &obj_buf, &obj_buf_sz); perf_clang__cleanup(); if (err) { + struct strbuf str_obj_buf = STRBUF_INIT; + pr_debug("bpf: builtin compilation failed: %d, try external compiler\n", err); - err = llvm__compile_bpf(filename, &obj_buf, &obj_buf_sz); - if (err) + err = llvm__compile_bpf(filename, &str_obj_buf); + if (err < 0) return ERR_PTR(-BPF_LOADER_ERRNO__COMPILE); + obj_buf = str_obj_buf.buf; + obj_buf_sz = str_obj_buf.len; } else pr_debug("bpf: successful builtin compilation\n"); obj = bpf_object__open_mem(obj_buf, obj_buf_sz, &opts); diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c index 4e8e243a6e4b..8a12160320f3 100644 --- a/tools/perf/util/llvm-utils.c +++ b/tools/perf/util/llvm-utils.c @@ -17,7 +17,9 @@ #include "config.h" #include "util.h" #include <sys/wait.h> +#include <api/strbuf.h> #include <subcmd/exec-cmd.h> +#include <subcmd/run-command.h> #define CLANG_BPF_CMD_DEFAULT_TEMPLATE \ "$CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS "\ @@ -125,92 +127,6 @@ static int search_program_and_warn(const char *def, const char *name, return ret; } -#define READ_SIZE 4096 -static int -read_from_pipe(const char *cmd, void **p_buf, size_t *p_read_sz) -{ - int err = 0; - void *buf = NULL; - FILE *file = NULL; - size_t read_sz = 0, buf_sz = 0; - char serr[STRERR_BUFSIZE]; - - file = popen(cmd, "r"); - if (!file) { - pr_err("ERROR: unable to popen cmd: %s\n", - str_error_r(errno, serr, sizeof(serr))); - return -EINVAL; - } - - while (!feof(file) && !ferror(file)) { - /* - * Make buf_sz always have obe byte extra space so we - * can put '\0' there. - */ - if (buf_sz - read_sz < READ_SIZE + 1) { - void *new_buf; - - buf_sz = read_sz + READ_SIZE + 1; - new_buf = realloc(buf, buf_sz); - - if (!new_buf) { - pr_err("ERROR: failed to realloc memory\n"); - err = -ENOMEM; - goto errout; - } - - buf = new_buf; - } - read_sz += fread(buf + read_sz, 1, READ_SIZE, file); - } - - if (buf_sz - read_sz < 1) { - pr_err("ERROR: internal error\n"); - err = -EINVAL; - goto errout; - } - - if (ferror(file)) { - pr_err("ERROR: error occurred when reading from pipe: %s\n", - str_error_r(errno, serr, sizeof(serr))); - err = -EIO; - goto errout; - } - - err = WEXITSTATUS(pclose(file)); - file = NULL; - if (err) { - err = -EINVAL; - goto errout; - } - - /* - * If buf is string, give it terminal '\0' to make our life - * easier. If buf is not string, that '\0' is out of space - * indicated by read_sz so caller won't even notice it. - */ - ((char *)buf)[read_sz] = '\0'; - - if (!p_buf) - free(buf); - else - *p_buf = buf; - - if (p_read_sz) - *p_read_sz = read_sz; - return 0; - -errout: - if (file) - pclose(file); - free(buf); - if (p_buf) - *p_buf = NULL; - if (p_read_sz) - *p_read_sz = 0; - return err; -} - static inline void force_set_env(const char *var, const char *value) { @@ -244,7 +160,7 @@ version_notice(void) ); } -static int detect_kbuild_dir(char **kbuild_dir) +static int detect_kbuild_dir(struct strbuf *kbuild_dir) { const char *test_dir = llvm_param.kbuild_dir; const char *prefix_dir = ""; @@ -276,10 +192,9 @@ static int detect_kbuild_dir(char **kbuild_dir) if (access(autoconf_path, R_OK) == 0) { free(autoconf_path); - err = asprintf(kbuild_dir, "%s%s%s", prefix_dir, test_dir, - suffix_dir); + err = (int)strbuf_addf(kbuild_dir, "%s%s%s", prefix_dir, test_dir, suffix_dir); if (err < 0) - return -ENOMEM; + return err; return 0; } pr_debug("%s: Couldn't find \"%s\", missing kernel-devel package?.\n", @@ -315,7 +230,7 @@ static const char *kinc_fetch_script = "rm -rf $TMPDIR\n" "exit $RET\n"; -void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts) +void llvm__get_kbuild_opts(struct strbuf *kbuild_dir, struct strbuf *kbuild_include_opts) { static char *saved_kbuild_dir; static char *saved_kbuild_include_opts; @@ -324,22 +239,14 @@ void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts) if (!kbuild_dir || !kbuild_include_opts) return; - *kbuild_dir = NULL; - *kbuild_include_opts = NULL; - if (saved_kbuild_dir && saved_kbuild_include_opts && !IS_ERR(saved_kbuild_dir) && !IS_ERR(saved_kbuild_include_opts)) { - *kbuild_dir = strdup(saved_kbuild_dir); - *kbuild_include_opts = strdup(saved_kbuild_include_opts); + strbuf_addstr(kbuild_dir, saved_kbuild_dir); + strbuf_addstr(kbuild_include_opts, saved_kbuild_include_opts); - if (*kbuild_dir && *kbuild_include_opts) - return; - - zfree(kbuild_dir); - zfree(kbuild_include_opts); /* - * Don't fall through: it may breaks saved_kbuild_dir and - * saved_kbuild_include_opts if detect them again when + * Don't fallthrough: it may break the saved_kbuild_dir and + * saved_kbuild_include_opts if they are detected again when * memory is low. */ return; @@ -361,28 +268,26 @@ void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts) goto errout; } - pr_debug("Kernel build dir is set to %s\n", *kbuild_dir); - force_set_env("KBUILD_DIR", *kbuild_dir); + pr_debug("Kernel build dir is set to %s\n", kbuild_dir->buf); + force_set_env("KBUILD_DIR", kbuild_dir->buf); force_set_env("KBUILD_OPTS", llvm_param.kbuild_opts); - err = read_from_pipe(kinc_fetch_script, - (void **)kbuild_include_opts, - NULL); + err = run_command_strbuf(kinc_fetch_script, kbuild_include_opts); if (err) { pr_warning( "WARNING:\tunable to get kernel include directories from '%s'\n" "Hint:\tTry set clang include options using 'clang-bpf-cmd-template'\n" " \toption in [llvm] section of ~/.perfconfig and set 'kbuild-dir'\n" " \toption in [llvm] to \"\" to suppress this detection.\n\n", - *kbuild_dir); + kbuild_dir->buf); zfree(kbuild_dir); goto errout; } - pr_debug("include option is set to %s\n", *kbuild_include_opts); + pr_debug("include option is set to %s\n", kbuild_include_opts->buf); - saved_kbuild_dir = strdup(*kbuild_dir); - saved_kbuild_include_opts = strdup(*kbuild_include_opts); + saved_kbuild_dir = strdup(kbuild_dir->buf); + saved_kbuild_include_opts = strdup(kbuild_include_opts->buf); if (!saved_kbuild_dir || !saved_kbuild_include_opts) { zfree(&saved_kbuild_dir); @@ -446,24 +351,23 @@ void llvm__dump_obj(const char *path, void *obj_buf, size_t size) free(obj_path); } -int llvm__compile_bpf(const char *path, void **p_obj_buf, - size_t *p_obj_buf_sz) +int llvm__compile_bpf(const char *path, struct strbuf *obj_buf) { - size_t obj_buf_sz; - void *obj_buf = NULL; int err, nr_cpus_avail; unsigned int kernel_version; char linux_version_code_str[64]; const char *clang_opt = llvm_param.clang_opt; char clang_path[PATH_MAX], llc_path[PATH_MAX], abspath[PATH_MAX], nr_cpus_avail_str[64]; char serr[STRERR_BUFSIZE]; - char *kbuild_dir = NULL, *kbuild_include_opts = NULL, - *perf_bpf_include_opts = NULL; + char *perf_bpf_include_opts = NULL; const char *template = llvm_param.clang_bpf_cmd_template; char *pipe_template = NULL; const char *opts = llvm_param.opts; - char *command_echo = NULL, *command_out; + char *command_echo = NULL; char *libbpf_include_dir = system_path(LIBBPF_INCLUDE_DIR); + struct strbuf kbuild_dir = STRBUF_INIT; + struct strbuf kbuild_include_opts = STRBUF_INIT; + struct strbuf command_out = STRBUF_INIT; if (path[0] != '-' && realpath(path, abspath) == NULL) { err = errno; @@ -501,9 +405,9 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf, force_set_env("LINUX_VERSION_CODE", linux_version_code_str); force_set_env("CLANG_EXEC", clang_path); force_set_env("CLANG_OPTIONS", clang_opt); - force_set_env("KERNEL_INC_OPTIONS", kbuild_include_opts); + force_set_env("KERNEL_INC_OPTIONS", kbuild_include_opts.buf); force_set_env("PERF_BPF_INC_OPTIONS", perf_bpf_include_opts); - force_set_env("WORKING_DIR", kbuild_dir ? : "."); + force_set_env("WORKING_DIR", kbuild_dir.len ? kbuild_dir.buf : "."); if (opts) { err = search_program_and_warn(llvm_param.llc_path, "llc", llc_path); @@ -549,11 +453,11 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf, SWAP_CHAR('&', '\006'); SWAP_CHAR('\a', '"'); } - err = read_from_pipe(command_echo, (void **) &command_out, NULL); - if (err) + err = run_command_strbuf(command_echo, &command_out); + if (err < 0) goto errout; - for (char *p = command_out; *p; p++) { + for (char *p = command_out.buf; *p; p++) { SWAP_CHAR('\001', '<'); SWAP_CHAR('\002', '>'); SWAP_CHAR('\003', '"'); @@ -562,10 +466,10 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf, SWAP_CHAR('\006', '&'); } #undef SWAP_CHAR - pr_debug("llvm compiling command : %s\n", command_out); + pr_debug("llvm compiling command : %s\n", command_out.buf); - err = read_from_pipe(template, &obj_buf, &obj_buf_sz); - if (err) { + err = run_command_strbuf(template, obj_buf); + if (err < 0) { pr_err("ERROR:\tunable to compile %s\n", path); pr_err("Hint:\tCheck error message shown above.\n"); pr_err("Hint:\tYou can also pre-compile it into .o using:\n"); @@ -574,33 +478,15 @@ int llvm__compile_bpf(const char *path, void **p_obj_buf, goto errout; } - free(command_echo); - free(command_out); - free(kbuild_dir); - free(kbuild_include_opts); - free(perf_bpf_include_opts); - free(libbpf_include_dir); - - if (!p_obj_buf) - free(obj_buf); - else - *p_obj_buf = obj_buf; - - if (p_obj_buf_sz) - *p_obj_buf_sz = obj_buf_sz; - return 0; + err = 0; errout: free(command_echo); - free(kbuild_dir); - free(kbuild_include_opts); - free(obj_buf); + strbuf_release(&command_out); + strbuf_release(&kbuild_dir); + strbuf_release(&kbuild_include_opts); free(perf_bpf_include_opts); free(libbpf_include_dir); free(pipe_template); - if (p_obj_buf) - *p_obj_buf = NULL; - if (p_obj_buf_sz) - *p_obj_buf_sz = 0; return err; } diff --git a/tools/perf/util/llvm-utils.h b/tools/perf/util/llvm-utils.h index 7878a0e3fa98..0ad25e42aa6e 100644 --- a/tools/perf/util/llvm-utils.h +++ b/tools/perf/util/llvm-utils.h @@ -56,13 +56,15 @@ struct llvm_param { extern struct llvm_param llvm_param; int perf_llvm_config(const char *var, const char *value); -int llvm__compile_bpf(const char *path, void **p_obj_buf, size_t *p_obj_buf_sz); +struct strbuf; +int llvm__compile_bpf(const char *path, struct strbuf *obj_buf); /* This function is for test__llvm() use only */ int llvm__search_clang(void); /* Following functions are reused by builtin clang support */ -void llvm__get_kbuild_opts(char **kbuild_dir, char **kbuild_include_opts); +struct strbuf; +void llvm__get_kbuild_opts(struct strbuf *kbuild_dir, struct strbuf *kbuild_include_opts); int llvm__get_nr_cpus(void); void llvm__dump_obj(const char *path, void *obj_buf, size_t size); -- 2.39.0.314.g84b9a713c41-goog