| Message ID | 20190322150105.24136-1-kaslevs@vmware.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | trace-cmd: Change tracecmd_msg's buf member type to char * | expand |
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> > ---
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
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
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) {
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(-)