Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Rikard Falkeborn <rikard.falkeborn@gmail.com>
To:     linux-kernel@vger.kernel.org
Cc:     acme@redhat.com, tstoyanov@vmware.com, rostedt@goodmis.org,
        rikard.falkeborn@gmail.com
Subject: [PATCH] tools lib traceevent: Fix missing equality check for strcmp
Date:   Tue,  9 Apr 2019 11:15:29 +0200
Message-Id: <20190409091529.2686-1-rikard.falkeborn@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

There was a missing comparison with 0 when checking if type is "s64" or
"u64". Therefore, the body of the if-statement was entered if "type" was
"u64" or not "s64", which made the first strcmp() redundant since if
type is "u64", it's not "s64".

If type is "s64", the body of the if-statement is not entered but since the
remainder of the function consists of if-statements which will not be
entered if type is "s64", we will just return "val", which is correct,
albeit at the cost of a few more calls to strcmp(), i.e., it will behave
just as if the if-statement was entered.

If type is neither "s64" or "u64", the body of the if-statement will be
entered incorrectly and "val" returned. This means that any type that is
checked after "s64" and "u64" is handled the same way as "s64" and
"u64", i.e., the limiting of "val" to fit in for example "s8" is never
reached.

This was introduced in the kernel tree when the sources were copied from
trace-cmd in commit f7d82350e597 ("tools/events: Add files to create
libtraceevent.a"), and in the trace-cmd repo in 1cdbae6035cei ("Implement
typecasting in parser") when the function was introduced, i.e., it has
always behaved the wrong way.

Detected by cppcheck.

Fixes: f7d82350e597 ("tools/events: Add files to create libtraceevent.a")
Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
I have only compile tested the patch but it should be correct. I don't
know if any other problems will surface due to this though.
---
 tools/lib/traceevent/event-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c
index 87494c7c619d..981c6ce2da2c 100644
--- a/tools/lib/traceevent/event-parse.c
+++ b/tools/lib/traceevent/event-parse.c
@@ -2233,7 +2233,7 @@ eval_type_str(unsigned long long val, const char *type, int pointer)
 		return val & 0xffffffff;
 
 	if (strcmp(type, "u64") == 0 ||
-	    strcmp(type, "s64"))
+	    strcmp(type, "s64") == 0)
 		return val;
 
 	if (strcmp(type, "s8") == 0)
-- 
2.21.0