| Message ID | 20231005230851.3666908-5-irogers@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clang-tools support in tools | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Thu, Oct 5, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote: > > Fix clang-tidy found potential memory leak and unread value: > ``` > tools/perf/util/hisi-ptt.c:108:3: warning: Value stored to 'data_offset' is never read [clang-analyzer-deadcode.DeadStores] > data_offset = 0; > ^ ~ > tools/perf/util/hisi-ptt.c:108:3: note: Value stored to 'data_offset' is never read > data_offset = 0; > ^ ~ > tools/perf/util/hisi-ptt.c:112:12: warning: Potential leak of memory pointed to by 'data' [clang-analyzer-unix.Malloc] > return -errno; > ^ > /usr/include/errno.h:38:18: note: expanded from macro 'errno' > ^ > tools/perf/util/hisi-ptt.c:100:15: note: Memory is allocated > void *data = malloc(size); > ^~~~~~~~~~~~ > tools/perf/util/hisi-ptt.c:104:6: note: Assuming 'data' is non-null > if (!data) > ^~~~~ > tools/perf/util/hisi-ptt.c:104:2: note: Taking false branch > if (!data) > ^ > tools/perf/util/hisi-ptt.c:107:6: note: Assuming the condition is false > if (perf_data__is_pipe(session->data)) { > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > tools/perf/util/hisi-ptt.c:107:2: note: Taking false branch > if (perf_data__is_pipe(session->data)) { > ^ > tools/perf/util/hisi-ptt.c:111:7: note: Assuming the condition is true > if (data_offset == -1) > ^~~~~~~~~~~~~~~~~ > tools/perf/util/hisi-ptt.c:111:3: note: Taking true branch > if (data_offset == -1) > ^ > tools/perf/util/hisi-ptt.c:112:12: note: Potential leak of memory pointed to by 'data' > return -errno; > ^ > /usr/include/errno.h:38:18: note: expanded from macro 'errno' > ``` We already have https://lore.kernel.org/r/20230930072719.1267784-1-visitorckw@gmail.com Thanks, Namhyung > > Signed-off-by: Ian Rogers <irogers@google.com> > --- > tools/perf/util/hisi-ptt.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c > index 45b614bb73bf..ea297329c526 100644 > --- a/tools/perf/util/hisi-ptt.c > +++ b/tools/perf/util/hisi-ptt.c > @@ -98,18 +98,18 @@ static int hisi_ptt_process_auxtrace_event(struct perf_session *session, > int fd = perf_data__fd(session->data); > int size = event->auxtrace.size; > void *data = malloc(size); > - off_t data_offset; > int err; > > if (!data) > return -errno; > > - if (perf_data__is_pipe(session->data)) { > - data_offset = 0; > - } else { > - data_offset = lseek(fd, 0, SEEK_CUR); > - if (data_offset == -1) > + if (!perf_data__is_pipe(session->data)) { > + off_t data_offset = lseek(fd, 0, SEEK_CUR); > + > + if (data_offset == -1) { > + free(data); > return -errno; > + } > } > > err = readn(fd, data, size); > -- > 2.42.0.609.gbb76f46606-goog >
On Sun, Oct 8, 2023 at 10:41 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Thu, Oct 5, 2023 at 4:09 PM Ian Rogers <irogers@google.com> wrote: > > > > Fix clang-tidy found potential memory leak and unread value: > > ``` > > tools/perf/util/hisi-ptt.c:108:3: warning: Value stored to 'data_offset' is never read [clang-analyzer-deadcode.DeadStores] > > data_offset = 0; > > ^ ~ > > tools/perf/util/hisi-ptt.c:108:3: note: Value stored to 'data_offset' is never read > > data_offset = 0; > > ^ ~ > > tools/perf/util/hisi-ptt.c:112:12: warning: Potential leak of memory pointed to by 'data' [clang-analyzer-unix.Malloc] > > return -errno; > > ^ > > /usr/include/errno.h:38:18: note: expanded from macro 'errno' > > ^ > > tools/perf/util/hisi-ptt.c:100:15: note: Memory is allocated > > void *data = malloc(size); > > ^~~~~~~~~~~~ > > tools/perf/util/hisi-ptt.c:104:6: note: Assuming 'data' is non-null > > if (!data) > > ^~~~~ > > tools/perf/util/hisi-ptt.c:104:2: note: Taking false branch > > if (!data) > > ^ > > tools/perf/util/hisi-ptt.c:107:6: note: Assuming the condition is false > > if (perf_data__is_pipe(session->data)) { > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > tools/perf/util/hisi-ptt.c:107:2: note: Taking false branch > > if (perf_data__is_pipe(session->data)) { > > ^ > > tools/perf/util/hisi-ptt.c:111:7: note: Assuming the condition is true > > if (data_offset == -1) > > ^~~~~~~~~~~~~~~~~ > > tools/perf/util/hisi-ptt.c:111:3: note: Taking true branch > > if (data_offset == -1) > > ^ > > tools/perf/util/hisi-ptt.c:112:12: note: Potential leak of memory pointed to by 'data' > > return -errno; > > ^ > > /usr/include/errno.h:38:18: note: expanded from macro 'errno' > > ``` > > We already have > > https://lore.kernel.org/r/20230930072719.1267784-1-visitorckw@gmail.com Agreed. There is a written to but not read addressed in this one, but I think that is okay to clean up later and to drop this patch. Thanks, Ian > Thanks, > Namhyung > > > > > > Signed-off-by: Ian Rogers <irogers@google.com> > > --- > > tools/perf/util/hisi-ptt.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c > > index 45b614bb73bf..ea297329c526 100644 > > --- a/tools/perf/util/hisi-ptt.c > > +++ b/tools/perf/util/hisi-ptt.c > > @@ -98,18 +98,18 @@ static int hisi_ptt_process_auxtrace_event(struct perf_session *session, > > int fd = perf_data__fd(session->data); > > int size = event->auxtrace.size; > > void *data = malloc(size); > > - off_t data_offset; > > int err; > > > > if (!data) > > return -errno; > > > > - if (perf_data__is_pipe(session->data)) { > > - data_offset = 0; > > - } else { > > - data_offset = lseek(fd, 0, SEEK_CUR); > > - if (data_offset == -1) > > + if (!perf_data__is_pipe(session->data)) { > > + off_t data_offset = lseek(fd, 0, SEEK_CUR); > > + > > + if (data_offset == -1) { > > + free(data); > > return -errno; > > + } > > } > > > > err = readn(fd, data, size); > > -- > > 2.42.0.609.gbb76f46606-goog > >
diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c index 45b614bb73bf..ea297329c526 100644 --- a/tools/perf/util/hisi-ptt.c +++ b/tools/perf/util/hisi-ptt.c @@ -98,18 +98,18 @@ static int hisi_ptt_process_auxtrace_event(struct perf_session *session, int fd = perf_data__fd(session->data); int size = event->auxtrace.size; void *data = malloc(size); - off_t data_offset; int err; if (!data) return -errno; - if (perf_data__is_pipe(session->data)) { - data_offset = 0; - } else { - data_offset = lseek(fd, 0, SEEK_CUR); - if (data_offset == -1) + if (!perf_data__is_pipe(session->data)) { + off_t data_offset = lseek(fd, 0, SEEK_CUR); + + if (data_offset == -1) { + free(data); return -errno; + } } err = readn(fd, data, size);
Fix clang-tidy found potential memory leak and unread value: ``` tools/perf/util/hisi-ptt.c:108:3: warning: Value stored to 'data_offset' is never read [clang-analyzer-deadcode.DeadStores] data_offset = 0; ^ ~ tools/perf/util/hisi-ptt.c:108:3: note: Value stored to 'data_offset' is never read data_offset = 0; ^ ~ tools/perf/util/hisi-ptt.c:112:12: warning: Potential leak of memory pointed to by 'data' [clang-analyzer-unix.Malloc] return -errno; ^ /usr/include/errno.h:38:18: note: expanded from macro 'errno' ^ tools/perf/util/hisi-ptt.c:100:15: note: Memory is allocated void *data = malloc(size); ^~~~~~~~~~~~ tools/perf/util/hisi-ptt.c:104:6: note: Assuming 'data' is non-null if (!data) ^~~~~ tools/perf/util/hisi-ptt.c:104:2: note: Taking false branch if (!data) ^ tools/perf/util/hisi-ptt.c:107:6: note: Assuming the condition is false if (perf_data__is_pipe(session->data)) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ tools/perf/util/hisi-ptt.c:107:2: note: Taking false branch if (perf_data__is_pipe(session->data)) { ^ tools/perf/util/hisi-ptt.c:111:7: note: Assuming the condition is true if (data_offset == -1) ^~~~~~~~~~~~~~~~~ tools/perf/util/hisi-ptt.c:111:3: note: Taking true branch if (data_offset == -1) ^ tools/perf/util/hisi-ptt.c:112:12: note: Potential leak of memory pointed to by 'data' return -errno; ^ /usr/include/errno.h:38:18: note: expanded from macro 'errno' ``` Signed-off-by: Ian Rogers <irogers@google.com> --- tools/perf/util/hisi-ptt.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)