| Message ID | 20190204070855.8921-7-kaslevs@vmware.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 1c6c5efd28a65dcefcd1d65cc474c7c7698b30ff |
| Headers | show |
| Series | trace-cmd protocol fixes | expand |
On Mon, 4 Feb 2019 09:08:53 +0200 Slavomir Kaslev <kaslevs@vmware.com> wrote: I accepted your patches up to here with some slight modifications to the subjects and change logs. Note, when referencing function names, its mostly desirable to add a "()" to the end of them to make it stand out that they are functions. Like tracecmd_msg_send_close(). But, this patch needs a change log to explain why this function should return an error code. Is something going to rely on it in the future? -- Steve > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > --- > include/trace-cmd/trace-cmd.h | 2 +- > tracecmd/trace-msg.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > index 33f352b..0ab23f6 100644 > --- a/include/trace-cmd/trace-cmd.h > +++ b/include/trace-cmd/trace-cmd.h > @@ -318,7 +318,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle, > int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle, > const char *buf, int size); > int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle); > -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); > +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); > > /* for server */ > int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle); > diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c > index b4b58d4..c24424b 100644 > --- a/tracecmd/trace-msg.c > +++ b/tracecmd/trace-msg.c > @@ -549,12 +549,12 @@ int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle, > return 0; > } > > -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) > +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) > { > struct tracecmd_msg msg; > > tracecmd_msg_init(MSG_CLOSE, &msg); > - tracecmd_msg_send(msg_handle->fd, &msg); > + return tracecmd_msg_send(msg_handle->fd, &msg); > } > > int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle,
On Tue, Feb 5, 2019 at 5:14 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > On Mon, 4 Feb 2019 09:08:53 +0200 > Slavomir Kaslev <kaslevs@vmware.com> wrote: > > I accepted your patches up to here with some slight modifications to > the subjects and change logs. Note, when referencing function names, > its mostly desirable to add a "()" to the end of them to make it stand > out that they are functions. Like tracecmd_msg_send_close(). > > But, this patch needs a change log to explain why this function should > return an error code. Is something going to rely on it in the future? Apologies for the empty commit message on this one, somehow it slipped me before sending. The intention for this patch was simply stylistic, as with close() which returns error code but it's seldom checked. Feel free to drop it if you find that unnecessary. Patch 7 and 8 don't depend on it. Should I resend them? -- Slavi > > -- Steve > > > > > Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> > > --- > > include/trace-cmd/trace-cmd.h | 2 +- > > tracecmd/trace-msg.c | 4 ++-- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h > > index 33f352b..0ab23f6 100644 > > --- a/include/trace-cmd/trace-cmd.h > > +++ b/include/trace-cmd/trace-cmd.h > > @@ -318,7 +318,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle, > > int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle, > > const char *buf, int size); > > int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle); > > -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); > > +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); > > > > /* for server */ > > int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle); > > diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c > > index b4b58d4..c24424b 100644 > > --- a/tracecmd/trace-msg.c > > +++ b/tracecmd/trace-msg.c > > @@ -549,12 +549,12 @@ int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle, > > return 0; > > } > > > > -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) > > +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) > > { > > struct tracecmd_msg msg; > > > > tracecmd_msg_init(MSG_CLOSE, &msg); > > - tracecmd_msg_send(msg_handle->fd, &msg); > > + return tracecmd_msg_send(msg_handle->fd, &msg); > > } > > > > int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle, >
On Thu, 7 Feb 2019 12:52:03 +0000 Slavomir Kaslev <kaslevs@vmware.com> wrote: > On Tue, Feb 5, 2019 at 5:14 PM Steven Rostedt <rostedt@goodmis.org> wrote: > > > > On Mon, 4 Feb 2019 09:08:53 +0200 > > Slavomir Kaslev <kaslevs@vmware.com> wrote: > > > > I accepted your patches up to here with some slight modifications to > > the subjects and change logs. Note, when referencing function names, > > its mostly desirable to add a "()" to the end of them to make it stand > > out that they are functions. Like tracecmd_msg_send_close(). > > > > But, this patch needs a change log to explain why this function should > > return an error code. Is something going to rely on it in the future? > > Apologies for the empty commit message on this one, somehow it slipped > me before sending. > > The intention for this patch was simply stylistic, as with close() > which returns error code but it's seldom checked. > > Feel free to drop it if you find that unnecessary. Patch 7 and 8 don't > depend on it. Should I resend them? > I'll drop this patch and apply the other two (after looking a bit more at them). No need to resend. Thanks, -- Steve
On Tue, 5 Feb 2019 10:14:49 -0500 Steven Rostedt <rostedt@goodmis.org> wrote: > On Mon, 4 Feb 2019 09:08:53 +0200 > Slavomir Kaslev <kaslevs@vmware.com> wrote: > > I accepted your patches up to here with some slight modifications to > the subjects and change logs. Note, when referencing function names, > its mostly desirable to add a "()" to the end of them to make it stand > out that they are functions. Like tracecmd_msg_send_close(). > > But, this patch needs a change log to explain why this function should > return an error code. Is something going to rely on it in the future? > I think I'll take this patch anyway (and add a change log to it). Since it can fail, it would be good to return the error. -- Steve
diff --git a/include/trace-cmd/trace-cmd.h b/include/trace-cmd/trace-cmd.h index 33f352b..0ab23f6 100644 --- a/include/trace-cmd/trace-cmd.h +++ b/include/trace-cmd/trace-cmd.h @@ -318,7 +318,7 @@ int tracecmd_msg_send_init_data(struct tracecmd_msg_handle *msg_handle, int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle, const char *buf, int size); int tracecmd_msg_finish_sending_data(struct tracecmd_msg_handle *msg_handle); -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle); /* for server */ int tracecmd_msg_initial_setting(struct tracecmd_msg_handle *msg_handle); diff --git a/tracecmd/trace-msg.c b/tracecmd/trace-msg.c index b4b58d4..c24424b 100644 --- a/tracecmd/trace-msg.c +++ b/tracecmd/trace-msg.c @@ -549,12 +549,12 @@ int tracecmd_msg_send_port_array(struct tracecmd_msg_handle *msg_handle, return 0; } -void tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) +int tracecmd_msg_send_close_msg(struct tracecmd_msg_handle *msg_handle) { struct tracecmd_msg msg; tracecmd_msg_init(MSG_CLOSE, &msg); - tracecmd_msg_send(msg_handle->fd, &msg); + return tracecmd_msg_send(msg_handle->fd, &msg); } int tracecmd_msg_data_send(struct tracecmd_msg_handle *msg_handle,
Signed-off-by: Slavomir Kaslev <kaslevs@vmware.com> --- include/trace-cmd/trace-cmd.h | 2 +- tracecmd/trace-msg.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)