diff mbox series

[v1,05/11] perf trace: Smatch: Fix potential NULL pointer dereference

Message ID 20190702103420.27540-6-leo.yan@linaro.org (mailing list archive)
State New, archived
Headers show
Series perf: Fix errors detected by Smatch | expand

Commit Message

Leo Yan July 2, 2019, 10:34 a.m. UTC
Based on the following report from Smatch, fix the potential
NULL pointer dereference check.

  tools/perf/builtin-trace.c:1044
  thread_trace__new() error: we previously assumed 'ttrace' could be
  null (see line 1041).

tools/perf/builtin-trace.c
1037 static struct thread_trace *thread_trace__new(void)
1038 {
1039         struct thread_trace *ttrace =  zalloc(sizeof(struct thread_trace));
1040
1041         if (ttrace)
1042                 ttrace->files.max = -1;
1043
1044         ttrace->syscall_stats = intlist__new(NULL);
             ^^^^^^^^
1045
1046         return ttrace;
1047 }

This patch directly returns NULL when fail to allocate memory for
ttrace; this can avoid potential NULL pointer dereference.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 tools/perf/builtin-trace.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Arnaldo Carvalho de Melo July 3, 2019, 6:46 p.m. UTC | #1
Em Tue, Jul 02, 2019 at 06:34:14PM +0800, Leo Yan escreveu:
> Based on the following report from Smatch, fix the potential
> NULL pointer dereference check.
> 
>   tools/perf/builtin-trace.c:1044
>   thread_trace__new() error: we previously assumed 'ttrace' could be
>   null (see line 1041).
> 
> tools/perf/builtin-trace.c
> 1037 static struct thread_trace *thread_trace__new(void)
> 1038 {
> 1039         struct thread_trace *ttrace =  zalloc(sizeof(struct thread_trace));
> 1040
> 1041         if (ttrace)
> 1042                 ttrace->files.max = -1;
> 1043
> 1044         ttrace->syscall_stats = intlist__new(NULL);
>              ^^^^^^^^
> 1045
> 1046         return ttrace;
> 1047 }
> 
> This patch directly returns NULL when fail to allocate memory for
> ttrace; this can avoid potential NULL pointer dereference.

I did it slightly different, to avoid having two return lines, and that
is what most other constructors (foo__new()) do in tools/perf/, kept
your changeset comment and patch authorship, thanks.

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index d0eb7224dd36..e3fc9062f136 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1038,10 +1038,10 @@ static struct thread_trace *thread_trace__new(void)
 {
 	struct thread_trace *ttrace =  zalloc(sizeof(struct thread_trace));
 
-	if (ttrace)
+	if (ttrace) {
 		ttrace->files.max = -1;
-
-	ttrace->syscall_stats = intlist__new(NULL);
+		ttrace->syscall_stats = intlist__new(NULL);
+	}
 
 	return ttrace;
 }
diff mbox series

Patch

diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index f3532b081b31..874d78890c60 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1038,9 +1038,10 @@  static struct thread_trace *thread_trace__new(void)
 {
 	struct thread_trace *ttrace =  zalloc(sizeof(struct thread_trace));
 
-	if (ttrace)
-		ttrace->files.max = -1;
+	if (ttrace == NULL)
+		return NULL;
 
+	ttrace->files.max = -1;
 	ttrace->syscall_stats = intlist__new(NULL);
 
 	return ttrace;