----- On Apr 18, 2019, at 12:18 PM, Yannick Lamarre ylama...@efficios.com wrote:
> lttng-ctl and lttng-sessiond use serialized communication for
> messages using lttng_event.
>
> Signed-off-by: Yannick Lamarre <ylama...@efficios.com>
> ---
> src/bin/lttng-sessiond/client.c | 23 ++++++++++++++++++++---
> src/common/sessiond-comm/sessiond-comm.h | 4 ++--
> src/lib/lttng-ctl/lttng-ctl.c | 14 ++++++++++----
> 3 files changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
> index 24e688c5..aed415ee 100644
> --- a/src/bin/lttng-sessiond/client.c
> +++ b/src/bin/lttng-sessiond/client.c
> @@ -1150,6 +1150,7 @@ error_add_context:
> case LTTNG_DISABLE_EVENT:
> {
>
> + struct lttng_event event;
I think we should remove the extra newline before the struct definition above
while we are there.
> /*
> * FIXME: handle filter; for now we just receive the filter's
> * bytecode along with the filter expression which are sent by
> @@ -1176,10 +1177,17 @@ error_add_context:
> count -= (size_t) ret;
> }
> }
> - /* FIXME: passing packed structure to non-packed pointer */
> + lttng_event_no_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + if (cmd_ctx->lsm->domain.type == LTTNG_DOMAIN_KERNEL) {
Those if() would be neater in a switch() case.
> + if (event.type == LTTNG_EVENT_PROBE || event.type ==
> LTTNG_EVENT_FUNCTION ||
> event.type == LTTNG_EVENT_USERSPACE_PROBE) {
> + lttng_event_probe_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + } else if (event.type == LTTNG_EVENT_FUNCTION_ENTRY) {
> + lttng_event_function_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + }
else what ? Error handling ?
if () else if () without a following else typically hides missing error
handling.
[...]
> - ev = lttng_event_copy(&cmd_ctx->lsm->u.enable.event);
> + lttng_event_no_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + if (cmd_ctx->lsm->domain.type == LTTNG_DOMAIN_KERNEL) {
> + if (event.type == LTTNG_EVENT_PROBE || event.type ==
> LTTNG_EVENT_FUNCTION ||
> event.type == LTTNG_EVENT_USERSPACE_PROBE) {
> + lttng_event_probe_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + } else if (event.type == LTTNG_EVENT_FUNCTION_ENTRY) {
> + lttng_event_function_attr_deserialize(&event,
> &cmd_ctx->lsm->u.disable.event);
> + }
same.
> + }
> + ev = lttng_event_copy(&event);
> if (!ev) {
> DBG("Failed to copy event: %s",
> cmd_ctx->lsm->u.enable.event.name);
> diff --git a/src/common/sessiond-comm/sessiond-comm.h
> b/src/common/sessiond-comm/sessiond-comm.h
> index 78b18185..4c2240a0 100644
> --- a/src/common/sessiond-comm/sessiond-comm.h
> +++ b/src/common/sessiond-comm/sessiond-comm.h
> @@ -370,7 +370,7 @@ struct lttcomm_session_msg {
> /* Event data */
> struct {
> char channel_name[LTTNG_SYMBOL_NAME_LEN];
> - struct lttng_event event LTTNG_PACKED;
> + struct lttng_event_serialized event;
> /* Length of following filter expression. */
> uint32_t expression_len;
> /* Length of following bytecode for filter. */
> @@ -389,7 +389,7 @@ struct lttcomm_session_msg {
> } LTTNG_PACKED enable;
> struct {
> char channel_name[LTTNG_SYMBOL_NAME_LEN];
> - struct lttng_event event LTTNG_PACKED;
> + struct lttng_event_serialized event;
> /* Length of following filter expression. */
> uint32_t expression_len;
> /* Length of following bytecode for filter. */
> diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
> index 686a98ef..a9adb5c2 100644
> --- a/src/lib/lttng-ctl/lttng-ctl.c
> +++ b/src/lib/lttng-ctl/lttng-ctl.c
> @@ -1110,8 +1110,15 @@ int lttng_enable_event_with_exclusions(struct
> lttng_handle *handle,
> }
>
> lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
> - /* FIXME: copying non-packed struct to packed struct. */
> - memcpy(&lsm.u.enable.event, ev, sizeof(lsm.u.enable.event));
> +
> + lttng_event_no_attr_serialize(&lsm.u.enable.event, ev);
> + if (handle->domain.type == LTTNG_DOMAIN_KERNEL) {
> + if (ev->type == LTTNG_EVENT_PROBE || ev->type ==
> LTTNG_EVENT_FUNCTION ||
> ev->type == LTTNG_EVENT_USERSPACE_PROBE) {
> + lttng_event_probe_attr_serialize(&lsm.u.enable.event,
> ev);
> + } else if (ev->type == LTTNG_EVENT_FUNCTION_ENTRY) {
> +
> lttng_event_function_attr_serialize(&lsm.u.enable.event, ev);
> + }
> + }
same.
Thanks,
Mathieu
>
> lttng_ctl_copy_string(lsm.session.name, handle->session_name,
> sizeof(lsm.session.name));
> @@ -1310,8 +1317,7 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
> lsm.cmd_type = LTTNG_DISABLE_EVENT;
>
> lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
> - /* FIXME: copying non-packed struct to packed struct. */
> - memcpy(&lsm.u.disable.event, ev, sizeof(lsm.u.disable.event));
> + lttng_event_no_attr_serialize(&lsm.u.disable.event, ev);
>
> lttng_ctl_copy_string(lsm.session.name, handle->session_name,
> sizeof(lsm.session.name));
> --
> 2.11.0
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
