[PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[Slavomir Kaslev]

Now that all protocol V3 messages (besides SEND_DATA) use the buffer for text,
it makes sense to have it declared as such and drop a few casts.

This also takes care of undefined behavior caused by void pointer arithmetic in
tracecmd_msg_data_send()/tracecmd_msg_read_data().

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 tracecmd/trace-msg.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index a91b211..9a27141 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -104,7 +104,7 @@ struct tracecmd_msg {
 		struct tracecmd_msg_tinit	tinit;
 		struct tracecmd_msg_rinit	rinit;
 	};
-	void					*buf;
+	char					*buf;
 } __attribute__((packed));
 
 static int msg_buf_len(struct tracecmd_msg *msg)
@@ -436,7 +436,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 		goto error;
 	}
 
-	if (((char *)msg.buf)[buf_len-1] != '\0') {
+	if (msg.buf[buf_len-1] != '\0') {
 		ret = -EINVAL;
 		goto error;
 	}
@@ -448,7 +448,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 		goto out;
 	}
 
-	buf_end = (char *)msg.buf + buf_len;
+	buf_end = msg.buf + buf_len;
 	for (i = 0, p = msg.buf; i < cpus; i++, p++) {
 		if (p >= buf_end) {
 			free(ports);
@@ -555,12 +555,12 @@ int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle)
 	if (buf_len == 0)
 		goto no_options;
 
-	if (((char *)msg.buf)[buf_len-1] != '\0') {
+	if (msg.buf[buf_len-1] != '\0') {
 		ret = -EINVAL;
 		goto error;
 	}
 
-	buf_end = (char *)msg.buf + buf_len;
+	buf_end = msg.buf + buf_len;
 	options = ntohl(msg.tinit.opt_num);
 	for (i = 0, p = msg.buf; i < options; i++, p++) {
 		if (p >= buf_end) {
-- 
2.19.1

[PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[Slavomir Kaslev]

Now that the buffer for protocol V3 messages is always text, it makes sense to
have it declared as such and drop a few casts.

Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
---
 tracecmd/trace-msg.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c
index a91b211..9a27141 100644
--- a/tracecmd/trace-msg.c
+++ b/tracecmd/trace-msg.c
@@ -104,7 +104,7 @@ struct tracecmd_msg {
 		struct tracecmd_msg_tinit	tinit;
 		struct tracecmd_msg_rinit	rinit;
 	};
-	void					*buf;
+	char					*buf;
 } __attribute__((packed));
 
 static int msg_buf_len(struct tracecmd_msg *msg)
@@ -436,7 +436,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 		goto error;
 	}
 
-	if (((char *)msg.buf)[buf_len-1] != '\0') {
+	if (msg.buf[buf_len-1] != '\0') {
 		ret = -EINVAL;
 		goto error;
 	}
@@ -448,7 +448,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle,
 		goto out;
 	}
 
-	buf_end = (char *)msg.buf + buf_len;
+	buf_end = msg.buf + buf_len;
 	for (i = 0, p = msg.buf; i < cpus; i++, p++) {
 		if (p >= buf_end) {
 			free(ports);
@@ -555,12 +555,12 @@ int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle)
 	if (buf_len == 0)
 		goto no_options;
 
-	if (((char *)msg.buf)[buf_len-1] != '\0') {
+	if (msg.buf[buf_len-1] != '\0') {
 		ret = -EINVAL;
 		goto error;
 	}
 
-	buf_end = (char *)msg.buf + buf_len;
+	buf_end = msg.buf + buf_len;
 	options = ntohl(msg.tinit.opt_num);
 	for (i = 0, p = msg.buf; i < options; i++, p++) {
 		if (p >= buf_end) {
-- 
2.19.1

Re: [PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[Steven Rostedt]

On Fri, 22 Mar 2019 17:01:05 +0200
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> Now that the buffer for protocol V3 messages is always text, it makes sense to
> have it declared as such and drop a few casts.

Is it always text?

When debugging the listener I saw the meta data sent as non text.

 tracecmd_msg_data_send()

Sends the data in binary, not text. Which is what we want.

-- Steve


> 
> Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com>
> ---

Re: [PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[Slavomir Kaslev]

On Fri, Mar 22, 2019 at 5:25 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Fri, 22 Mar 2019 17:01:05 +0200
> Slavomir Kaslev <kaslevs@vmware.com> wrote:
>
> > Now that the buffer for protocol V3 messages is always text, it makes sense to
> > have it declared as such and drop a few casts.
>
> Is it always text?
>
> When debugging the listener I saw the meta data sent as non text.
>
>  tracecmd_msg_data_send()
>
> Sends the data in binary, not text. Which is what we want.

You're completely right. I still think that it makes sense to have
that patch go in, maybe with rewritten commit message.
In tracecmd_msg_read_data()[1] we do pointer arithmetic on void
pointer which is undefined behavior (though gcc allows it unless
-pedantic-errors is passed) and this patch will take care of it.

Should I resend or drop it?

[1] https://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git/tree/tracecmd/trace-msg.c#n665

-- Slavi

Re: [PATCH] trace-cmd: Change tracecmd_msg's buf member type to char *

[Steven Rostedt]

On Fri, 22 Mar 2019 15:39:11 +0000
Slavomir Kaslev <kaslevs@vmware.com> wrote:

> You're completely right. I still think that it makes sense to have
> that patch go in, maybe with rewritten commit message.
> In tracecmd_msg_read_data()[1] we do pointer arithmetic on void
> pointer which is undefined behavior (though gcc allows it unless
> -pedantic-errors is passed) and this patch will take care of it.
> 
> Should I resend or drop it?

I think the patch cleans up the code (removes a lot of the type
casting). Resend it with a new change log.

-- Steve