Messages in this thread | | | Subject | Re: [PATCH v2 3/3] perf test: Decompress kernel module before objdump | From | Adrian Hunter <> | Date | Mon, 5 Jun 2017 14:13:27 +0300 |
| |
On 03/06/17 05:00, Namhyung Kim wrote: > If a kernel modules is compressed, it should be decompressed before > running objdump to parse binary data correctly. This fixes a failure of > object code reading test for me.
Makes sense to me although I have a few small comments below.
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/tests/code-reading.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c > index 1f14e7612cbb..fe6bd34d68b1 100644 > --- a/tools/perf/tests/code-reading.c > +++ b/tools/perf/tests/code-reading.c > @@ -229,6 +229,7 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode, > unsigned char buf2[BUFSZ]; > size_t ret_len; > u64 objdump_addr; > + const char *objdump_name; > int ret; > > pr_debug("Reading object code for memory address: %#"PRIx64"\n", addr); > @@ -289,9 +290,32 @@ static int read_object_code(u64 addr, size_t len, u8 cpumode, > state->done[state->done_cnt++] = al.map->start; > } > > + objdump_name = al.map->dso->long_name; > + if (dso__needs_decompress(al.map->dso)) { > + char buf[32]; /* for "/tmp/perf-kmod-XXXXXX" */
We should be able to set the temporary file name buffer size without reference to the format of the name i.e. #define it or use PATH_MAX
> + > + if (dso__decompress_kmodule_path(al.map->dso, objdump_name, > + buf, sizeof(buf)) < 0) {
Do we care that it is kmodule i.e. why isn't this function called just dso__decompress_path().
> + pr_debug("decompression failed\n"); > + return -1; > + } > + > + objdump_name = strdup(buf);
Why not just put the buf in the outer scope instead of using strdup().
> + if (objdump_name == NULL) { > + pr_debug("memory allocation failed\n"); > + return -1; > + } > + } > + > /* Read the object code using objdump */ > objdump_addr = map__rip_2objdump(al.map, al.addr); > - ret = read_via_objdump(al.map->dso->long_name, objdump_addr, buf2, len); > + ret = read_via_objdump(objdump_name, objdump_addr, buf2, len); > + > + if (objdump_name != al.map->dso->long_name) { > + unlink(objdump_name); > + free((void *)objdump_name); > + } > + > if (ret > 0) { > /* > * The kernel maps are inaccurate - assume objdump is right in >
| |