[PATCH lttng-ust 1/7] Define a new command LTTNG_UST_EXCLUSION

[PATCH lttng-ust 1/7] Define a new command LTTNG_UST_EXCLUSION

[JP Ikaheimonen]

Define a new command id for attaching exclusions to enablers.
Define the structure for passing the command.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 include/lttng/ust-abi.h       | 1 +
 include/ust-comm.h            | 3 +++
 liblttng-ust/lttng-ust-comm.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h
index 6630332..ab70d57 100644
--- a/include/lttng/ust-abi.h
+++ b/include/lttng/ust-abi.h
@@ -273,6 +273,7 @@ struct lttng_ust_filter_bytecode {
 
 /* Event FD commands */
 #define LTTNG_UST_FILTER			_UST_CMD(0xA0)
+#define LTTNG_UST_EXCLUSION			_UST_CMD(0xA1)
 
 #define LTTNG_UST_ROOT_HANDLE	0
 
diff --git a/include/ust-comm.h b/include/ust-comm.h
index ae53f15..b9bbb39 100644
--- a/include/ust-comm.h
+++ b/include/ust-comm.h
@@ -91,6 +91,9 @@ struct ustcomm_ust_msg {
 			uint32_t reloc_offset;
 			uint64_t seqnum;
 		} LTTNG_PACKED filter;
+		struct {
+			uint32_t count;	/* how many names follow */
+		} LTTNG_PACKED exclusion;
 		char padding[USTCOMM_MSG_PADDING2];
 	} u;
 } LTTNG_PACKED;
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index a6e4ba3..bf2750c 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -172,6 +172,7 @@ static const char *cmd_name_mapping[] = {
 
 	/* Event FD commands */
 	[ LTTNG_UST_FILTER ] = "Create Filter",
+	[ LTTNG_UST_EXCLUSION ] = "Add exclusions to event",
 };
 
 static const char *str_timeout;
-- 
1.8.1.2

[PATCH lttng-ust 2/7] Define exclusion structure for UST ABI

[JP Ikaheimonen]

Define a structure for holding exception data. The last data item
is a flexible array that contains a variable number of
exclusions names.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 include/lttng/ust-abi.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h
index ab70d57..0287466 100644
--- a/include/lttng/ust-abi.h
+++ b/include/lttng/ust-abi.h
@@ -228,6 +228,13 @@ struct lttng_ust_filter_bytecode {
 	char data[0];
 } LTTNG_PACKED;
 
+#define LTTNG_UST_EXCLUSION_PADDING	32
+struct lttng_ust_event_exclusion {
+	uint32_t count;
+	char padding[LTTNG_UST_EXCLUSION_PADDING];
+	char names[LTTNG_UST_SYM_NAME_LEN][0];
+} LTTNG_PACKED;
+
 #define _UST_CMD(minor)				(minor)
 #define _UST_CMDR(minor, type)			(minor)
 #define _UST_CMDW(minor, type)			(minor)
-- 
1.8.1.2

[PATCH lttng-ust 3/7] Send exclusion data through ustcomm

[JP Ikaheimonen]

Define a function ustctl_set_exclusion that sends the
exclusion data through ustcomm with the LTTNG_UST_EXCLUSION
command message.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 include/lttng/ust-ctl.h   |  2 ++
 liblttng-ust-ctl/ustctl.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/include/lttng/ust-ctl.h b/include/lttng/ust-ctl.h
index c8ac3f9..40e831e 100644
--- a/include/lttng/ust-ctl.h
+++ b/include/lttng/ust-ctl.h
@@ -73,6 +73,8 @@ int ustctl_add_context(int sock, struct lttng_ust_context *ctx,
 		struct lttng_ust_object_data **context_data);
 int ustctl_set_filter(int sock, struct lttng_ust_filter_bytecode *bytecode,
 		struct lttng_ust_object_data *obj_data);
+int ustctl_set_exclusion(int sock, struct lttng_ust_event_exclusion *exclusion,
+		struct lttng_ust_object_data *obj_data);
 
 int ustctl_enable(int sock, struct lttng_ust_object_data *object);
 int ustctl_disable(int sock, struct lttng_ust_object_data *object);
diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c
index 11fc704..00d9802 100644
--- a/liblttng-ust-ctl/ustctl.c
+++ b/liblttng-ust-ctl/ustctl.c
@@ -273,6 +273,40 @@ int ustctl_set_filter(int sock, struct lttng_ust_filter_bytecode *bytecode,
 	return ustcomm_recv_app_reply(sock, &lur, lum.handle, lum.cmd);
 }
 
+int ustctl_set_exclusion(int sock, struct lttng_ust_event_exclusion *exclusion,
+		struct lttng_ust_object_data *obj_data)
+{
+	struct ustcomm_ust_msg lum;
+	struct ustcomm_ust_reply lur;
+	int ret;
+
+	if (!obj_data) {
+		return -EINVAL;
+	}
+
+	memset(&lum, 0, sizeof(lum));
+	lum.handle = obj_data->handle;
+	lum.cmd = LTTNG_UST_EXCLUSION;
+	lum.u.exclusion.count = exclusion->count;
+
+	ret = ustcomm_send_app_msg(sock, &lum);
+	if (ret) {
+		return ret;
+	}
+
+	/* send var len bytecode */
+	ret = ustcomm_send_unix_sock(sock,
+			exclusion->names,
+			exclusion->count * LTTNG_UST_SYM_NAME_LEN);
+	if (ret < 0) {
+		return ret;
+	}
+	if (ret != exclusion->count * LTTNG_UST_SYM_NAME_LEN) {
+		return -EINVAL;
+	}
+	return ustcomm_recv_app_reply(sock, &lur, lum.handle, lum.cmd);
+}
+
 /* Enable event, channel and session ioctl */
 int ustctl_enable(int sock, struct lttng_ust_object_data *object)
 {
-- 
1.8.1.2

[PATCH lttng-ust 4/7] Add excluders to enabler structure

[JP Ikaheimonen]

Define a structure that holds excluders and can be used in lists.
Add a list holding these structures to the enabler structure.
Initialize and destroy the list when the enabler is initialized
and destroyed.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 include/lttng/ust-events.h  | 11 +++++++++++
 liblttng-ust/lttng-events.c |  8 ++++++++
 2 files changed, 19 insertions(+)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index c563511..db8b9dc 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -301,6 +301,8 @@ struct lttng_enabler {
 
 	/* head list of struct lttng_ust_filter_bytecode_node */
 	struct cds_list_head filter_bytecode_head;
+	/* head list of struct lttng_ust_excluder_node */
+	struct cds_list_head excluder_head;
 	struct cds_list_head node;	/* per-session list of enablers */
 
 	struct lttng_ust_event event_param;
@@ -342,6 +344,15 @@ struct lttng_ust_filter_bytecode_node {
 	struct lttng_ust_filter_bytecode bc;
 };
 
+struct lttng_ust_excluder_node {
+	struct cds_list_head node;
+	struct lttng_enabler *enabler;
+	/*
+	 * struct lttng_ust_event_exclusion had variable sized array,
+	 * must be last field.
+	 */
+	struct lttng_ust_event_exclusion excluder;
+};
 /*
  * Filter return value masks.
  */
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 26601a6..ad374f3 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -687,6 +687,7 @@ struct lttng_enabler *lttng_enabler_create(enum lttng_enabler_type type,
 		return NULL;
 	enabler->type = type;
 	CDS_INIT_LIST_HEAD(&enabler->filter_bytecode_head);
+	CDS_INIT_LIST_HEAD(&enabler->excluder_head);
 	memcpy(&enabler->event_param, event_param,
 		sizeof(enabler->event_param));
 	enabler->chan = chan;
@@ -767,6 +768,7 @@ static
 void lttng_enabler_destroy(struct lttng_enabler *enabler)
 {
 	struct lttng_ust_filter_bytecode_node *filter_node, *tmp_filter_node;
+	struct lttng_ust_excluder_node *excluder_node, *tmp_excluder_node;
 
 	/* Destroy filter bytecode */
 	cds_list_for_each_entry_safe(filter_node, tmp_filter_node,
@@ -774,6 +776,12 @@ void lttng_enabler_destroy(struct lttng_enabler *enabler)
 		free(filter_node);
 	}
 
+	/* Destroy excluders */
+	cds_list_for_each_entry_safe(excluder_node, tmp_excluder_node,
+			&enabler->excluder_head, node) {
+		free(excluder_node);
+	}
+
 	/* Destroy contexts */
 	lttng_destroy_context(enabler->ctx);
 
-- 
1.8.1.2

[PATCH lttng-ust 5/7] Store exclusions to enablers

[JP Ikaheimonen]

Implement a function that adds exclusions to the list in the enabler.
Call this function in the enabler command handler when the
LTTNG_UST_EXCLUSION command is received.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 include/lttng/ust-events.h   | 2 ++
 liblttng-ust/lttng-events.c  | 9 +++++++++
 liblttng-ust/lttng-ust-abi.c | 7 +++++++
 3 files changed, 18 insertions(+)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index db8b9dc..5d43570 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -549,6 +549,8 @@ int lttng_enabler_attach_bytecode(struct lttng_enabler *enabler,
 		struct lttng_ust_filter_bytecode_node *bytecode);
 int lttng_enabler_attach_context(struct lttng_enabler *enabler,
 		struct lttng_ust_context *ctx);
+int lttng_enabler_attach_exclusion(struct lttng_enabler *enabler,
+		struct lttng_ust_excluder_node *excluder);
 
 int lttng_attach_context(struct lttng_ust_context *context_param,
 		struct lttng_ctx **ctx, struct lttng_session *session);
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index ad374f3..915cffb 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -721,6 +721,15 @@ int lttng_enabler_attach_bytecode(struct lttng_enabler *enabler,
 	return 0;
 }
 
+int lttng_enabler_attach_exclusion(struct lttng_enabler *enabler,
+		struct lttng_ust_excluder_node *excluder)
+{
+	excluder->enabler = enabler;
+	cds_list_add_tail(&excluder->node, &enabler->excluder_head);
+	lttng_session_lazy_sync_enablers(enabler->chan->session);
+	return 0;
+}
+
 int lttng_attach_context(struct lttng_ust_context *context_param,
 		struct lttng_ctx **ctx, struct lttng_session *session)
 {
diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c
index a852aae..61245a8 100644
--- a/liblttng-ust/lttng-ust-abi.c
+++ b/liblttng-ust/lttng-ust-abi.c
@@ -933,6 +933,8 @@ static const struct lttng_ust_objd_ops lttng_channel_ops = {
  *		Disable recording for this enabler
  *	LTTNG_UST_FILTER
  *		Attach a filter to an enabler.
+ *	LTTNG_UST_EXCLUSION
+ *		Attach exclusions to an enabler.
  */
 static
 long lttng_enabler_cmd(int objd, unsigned int cmd, unsigned long arg,
@@ -958,6 +960,11 @@ long lttng_enabler_cmd(int objd, unsigned int cmd, unsigned long arg,
 			return ret;
 		return 0;
 	}
+	case LTTNG_UST_EXCLUSION:
+	{
+		return lttng_enabler_attach_exclusion(enabler,
+				(struct lttng_ust_excluder_node *) arg);
+	}
 	default:
 		return -EINVAL;
 	}
-- 
1.8.1.2

[PATCH lttng-ust 6/7] Add handler for LTTNG_UST_EXCLUSION in UST ABI

[JP Ikaheimonen]

Add message handler for the LTTNG_UST_EXCLUSION command in
UST ABI. Copy the exclusion data into a lttng_ust_excluder_node
structure and pass it to the enabler command handler.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 liblttng-ust/lttng-ust-comm.c | 62 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index bf2750c..597712b 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -488,6 +488,68 @@ int handle_message(struct sock_info *sock_info,
 		}
 		break;
 	}
+	case LTTNG_UST_EXCLUSION:
+	{
+		/* Receive exclusion names */
+		struct lttng_ust_excluder_node *node;
+		unsigned int count;
+
+		count = lum->u.exclusion.count;
+		if (count == 0) {
+			/* There are no names to read */
+			ret = 0;
+			goto error;
+		}
+		node = zmalloc(sizeof(*node) +
+				count * LTTNG_UST_SYM_NAME_LEN);
+		if (!node) {
+			ret = -ENOMEM;
+			goto error;
+		}
+		node->excluder.count = count;
+		len = ustcomm_recv_unix_sock(sock, node->excluder.names,
+				count * LTTNG_UST_SYM_NAME_LEN);
+		switch (len) {
+		case 0:	/* orderly shutdown */
+			ret = 0;
+			free(node);
+			goto error;
+		default:
+			if (len == count * LTTNG_UST_SYM_NAME_LEN) {
+				DBG("Exclusion data received");
+				break;
+			} else if (len < 0) {
+				DBG("Receive failed from lttng-sessiond with errno %d", (int) -len);
+				if (len == -ECONNRESET) {
+					ERR("%s remote end closed connection", sock_info->name);
+					ret = len;
+					free(node);
+					goto error;
+				}
+				ret = len;
+				free(node);
+				goto end;
+			} else {
+				DBG("Incorrect exclusion data message size: %zd", len);
+				ret = -EINVAL;
+				free(node);
+				goto end;
+			}
+		}
+		if (ops->cmd) {
+			ret = ops->cmd(lum->handle, lum->cmd,
+					(unsigned long) node,
+					&args, sock_info);
+			if (ret) {
+				free(node);
+			}
+			/* Don't free exclusion data if everything went fine. */
+		} else {
+			ret = -ENOSYS;
+			free(node);
+		}
+		break;
+	}
 	case LTTNG_UST_CHANNEL:
 	{
 		void *chan_data;
-- 
1.8.1.2

[PATCH lttng-ust 7/7] Add a check against excluders

[JP Ikaheimonen]

When matching enablers with events, first check against all
excluders of the enabler. If the event matches with any of the excluders,
then the event does not match with the enabler.

Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
---
 liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 915cffb..fa5596c 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -491,6 +491,29 @@ static
 int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
 		struct lttng_enabler *enabler)
 {
+	struct lttng_ust_excluder_node *excluder;
+
+	/* If event matches with an excluder, return 'does not match' */
+	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
+		int found = 0;
+		int count, len;
+		char *excluder_name;
+
+		for (count = 0; count < excluder->excluder.count; count++) {
+			excluder_name = (char *)(excluder->excluder.names)
+					+ count * LTTNG_UST_SYM_NAME_LEN;
+			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
+			if (len > 0 && excluder_name[len - 1] == '*') {
+				found = strncmp(desc->name, excluder_name, len - 1);
+			} else {
+				found = strncmp(desc->name, excluder_name,
+						LTTNG_UST_SYM_NAME_LEN - 1);
+			}
+			if (!found) {
+				return 0;
+			}
+		}
+	}
 	switch (enabler->type) {
 	case LTTNG_ENABLER_WILDCARD:
 		return lttng_desc_match_wildcard_enabler(desc, enabler);
-- 
1.8.1.2

Re: [PATCH lttng-ust 7/7] Add a check against excluders

[Mathieu Desnoyers]

----- Original Message -----
> From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> To: lttng-dev@lists.lttng.org
> Sent: Thursday, November 7, 2013 5:22:35 AM
> Subject: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders
> 
> When matching enablers with events, first check against all
> excluders of the enabler. If the event matches with any of the excluders,
> then the event does not match with the enabler.
> 
> Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
> ---
>  liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 915cffb..fa5596c 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -491,6 +491,29 @@ static
>  int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
>  		struct lttng_enabler *enabler)
>  {
> +	struct lttng_ust_excluder_node *excluder;
> +
> +	/* If event matches with an excluder, return 'does not match' */
> +	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
> +		int found = 0;

Why is "found" declared here ? Why is it initialized to 0 ?


> +		int count, len;
> +		char *excluder_name;
> +
> +		for (count = 0; count < excluder->excluder.count; count++) {

Rather than declared here ?


> +			excluder_name = (char *)(excluder->excluder.names)
> +					+ count * LTTNG_UST_SYM_NAME_LEN;
> +			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
> +			if (len > 0 && excluder_name[len - 1] == '*') {
> +				found = strncmp(desc->name, excluder_name, len - 1);

strncmp and "found" have reversed logic. I'd assign found = !strncmp(....).

> +			} else {
> +				found = strncmp(desc->name, excluder_name,
> +						LTTNG_UST_SYM_NAME_LEN - 1);
> +			}
> +			if (!found) {

and reverse this condition.

The other patches in this patchset look good !! Good job !!

Thanks,

Mathieu

> +				return 0;
> +			}
> +		}
> +	}
>  	switch (enabler->type) {
>  	case LTTNG_ENABLER_WILDCARD:
>  		return lttng_desc_match_wildcard_enabler(desc, enabler);
> --
> 1.8.1.2
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-ust 7/7] Add a check against excluders

[Mathieu Desnoyers]

Due to timing constaints for pulling into ust and tools in sync, I applied the modifications to your patch.

All 7 patches pulled into UST master.

Thanks!

Mathieu

----- Original Message -----
> From: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> To: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Wednesday, November 13, 2013 9:05:13 AM
> Subject: Re: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders
> 
> ----- Original Message -----
> > From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> > To: lttng-dev@lists.lttng.org
> > Sent: Thursday, November 7, 2013 5:22:35 AM
> > Subject: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders
> > 
> > When matching enablers with events, first check against all
> > excluders of the enabler. If the event matches with any of the excluders,
> > then the event does not match with the enabler.
> > 
> > Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
> > ---
> >  liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> > index 915cffb..fa5596c 100644
> > --- a/liblttng-ust/lttng-events.c
> > +++ b/liblttng-ust/lttng-events.c
> > @@ -491,6 +491,29 @@ static
> >  int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
> >  		struct lttng_enabler *enabler)
> >  {
> > +	struct lttng_ust_excluder_node *excluder;
> > +
> > +	/* If event matches with an excluder, return 'does not match' */
> > +	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
> > +		int found = 0;
> 
> Why is "found" declared here ? Why is it initialized to 0 ?
> 
> 
> > +		int count, len;
> > +		char *excluder_name;
> > +
> > +		for (count = 0; count < excluder->excluder.count; count++) {
> 
> Rather than declared here ?
> 
> 
> > +			excluder_name = (char *)(excluder->excluder.names)
> > +					+ count * LTTNG_UST_SYM_NAME_LEN;
> > +			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
> > +			if (len > 0 && excluder_name[len - 1] == '*') {
> > +				found = strncmp(desc->name, excluder_name, len - 1);
> 
> strncmp and "found" have reversed logic. I'd assign found = !strncmp(....).
> 
> > +			} else {
> > +				found = strncmp(desc->name, excluder_name,
> > +						LTTNG_UST_SYM_NAME_LEN - 1);
> > +			}
> > +			if (!found) {
> 
> and reverse this condition.
> 
> The other patches in this patchset look good !! Good job !!
> 
> Thanks,
> 
> Mathieu
> 
> > +				return 0;
> > +			}
> > +		}
> > +	}
> >  	switch (enabler->type) {
> >  	case LTTNG_ENABLER_WILDCARD:
> >  		return lttng_desc_match_wildcard_enabler(desc, enabler);
> > --
> > 1.8.1.2
> > 
> > 
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > http://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
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-ust 7/7] Add a check against excluders

[Ikaheimonen, JP]

Thank you Mathieu!
I will make these changes and resubmit this one patch.

Cheers,
JP

-----Original Message-----
From: Mathieu Desnoyers [mailto:mathieu.desnoyers@efficios.com] 
Sent: 13. marraskuuta 2013 16:05
To: Ikaheimonen, JP
Cc: lttng-dev@lists.lttng.org
Subject: Re: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders

----- Original Message -----
> From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> To: lttng-dev@lists.lttng.org
> Sent: Thursday, November 7, 2013 5:22:35 AM
> Subject: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against 
> excluders
> 
> When matching enablers with events, first check against all excluders 
> of the enabler. If the event matches with any of the excluders, then 
> the event does not match with the enabler.
> 
> Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
> ---
>  liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c 
> index 915cffb..fa5596c 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -491,6 +491,29 @@ static
>  int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
>  		struct lttng_enabler *enabler)
>  {
> +	struct lttng_ust_excluder_node *excluder;
> +
> +	/* If event matches with an excluder, return 'does not match' */
> +	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
> +		int found = 0;

Why is "found" declared here ? Why is it initialized to 0 ?


> +		int count, len;
> +		char *excluder_name;
> +
> +		for (count = 0; count < excluder->excluder.count; count++) {

Rather than declared here ?


> +			excluder_name = (char *)(excluder->excluder.names)
> +					+ count * LTTNG_UST_SYM_NAME_LEN;
> +			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
> +			if (len > 0 && excluder_name[len - 1] == '*') {
> +				found = strncmp(desc->name, excluder_name, len - 1);

strncmp and "found" have reversed logic. I'd assign found = !strncmp(....).

> +			} else {
> +				found = strncmp(desc->name, excluder_name,
> +						LTTNG_UST_SYM_NAME_LEN - 1);
> +			}
> +			if (!found) {

and reverse this condition.

The other patches in this patchset look good !! Good job !!

Thanks,

Mathieu

> +				return 0;
> +			}
> +		}
> +	}
>  	switch (enabler->type) {
>  	case LTTNG_ENABLER_WILDCARD:
>  		return lttng_desc_match_wildcard_enabler(desc, enabler);
> --
> 1.8.1.2
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-ust 7/7] Add a check against excluders

[Mathieu Desnoyers]

Not needed, I already fixed it.

Thanks,

Mathieu

----- Original Message -----
> From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Thursday, November 14, 2013 2:26:33 AM
> Subject: RE: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders
> 
> Thank you Mathieu!
> I will make these changes and resubmit this one patch.
> 
> Cheers,
> JP
> 
> -----Original Message-----
> From: Mathieu Desnoyers [mailto:mathieu.desnoyers@efficios.com]
> Sent: 13. marraskuuta 2013 16:05
> To: Ikaheimonen, JP
> Cc: lttng-dev@lists.lttng.org
> Subject: Re: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders
> 
> ----- Original Message -----
> > From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> > To: lttng-dev@lists.lttng.org
> > Sent: Thursday, November 7, 2013 5:22:35 AM
> > Subject: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against
> > excluders
> > 
> > When matching enablers with events, first check against all excluders
> > of the enabler. If the event matches with any of the excluders, then
> > the event does not match with the enabler.
> > 
> > Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
> > ---
> >  liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> > index 915cffb..fa5596c 100644
> > --- a/liblttng-ust/lttng-events.c
> > +++ b/liblttng-ust/lttng-events.c
> > @@ -491,6 +491,29 @@ static
> >  int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
> >  		struct lttng_enabler *enabler)
> >  {
> > +	struct lttng_ust_excluder_node *excluder;
> > +
> > +	/* If event matches with an excluder, return 'does not match' */
> > +	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
> > +		int found = 0;
> 
> Why is "found" declared here ? Why is it initialized to 0 ?
> 
> 
> > +		int count, len;
> > +		char *excluder_name;
> > +
> > +		for (count = 0; count < excluder->excluder.count; count++) {
> 
> Rather than declared here ?
> 
> 
> > +			excluder_name = (char *)(excluder->excluder.names)
> > +					+ count * LTTNG_UST_SYM_NAME_LEN;
> > +			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
> > +			if (len > 0 && excluder_name[len - 1] == '*') {
> > +				found = strncmp(desc->name, excluder_name, len - 1);
> 
> strncmp and "found" have reversed logic. I'd assign found = !strncmp(....).
> 
> > +			} else {
> > +				found = strncmp(desc->name, excluder_name,
> > +						LTTNG_UST_SYM_NAME_LEN - 1);
> > +			}
> > +			if (!found) {
> 
> and reverse this condition.
> 
> The other patches in this patchset look good !! Good job !!
> 
> Thanks,
> 
> Mathieu
> 
> > +				return 0;
> > +			}
> > +		}
> > +	}
> >  	switch (enabler->type) {
> >  	case LTTNG_ENABLER_WILDCARD:
> >  		return lttng_desc_match_wildcard_enabler(desc, enabler);
> > --
> > 1.8.1.2
> > 
> > 
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> > 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

Re: [PATCH lttng-ust 7/7] Add a check against excluders

[Ikaheimonen, JP]

Thank you!
JP

-----Original Message-----
From: Mathieu Desnoyers [mailto:mathieu.desnoyers@efficios.com] 
Sent: 14. marraskuuta 2013 16:25
To: Ikaheimonen, JP
Cc: lttng-dev@lists.lttng.org
Subject: Re: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against excluders

Not needed, I already fixed it.

Thanks,

Mathieu

----- Original Message -----
> From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> To: "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Cc: lttng-dev@lists.lttng.org
> Sent: Thursday, November 14, 2013 2:26:33 AM
> Subject: RE: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against 
> excluders
> 
> Thank you Mathieu!
> I will make these changes and resubmit this one patch.
> 
> Cheers,
> JP
> 
> -----Original Message-----
> From: Mathieu Desnoyers [mailto:mathieu.desnoyers@efficios.com]
> Sent: 13. marraskuuta 2013 16:05
> To: Ikaheimonen, JP
> Cc: lttng-dev@lists.lttng.org
> Subject: Re: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against 
> excluders
> 
> ----- Original Message -----
> > From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> > To: lttng-dev@lists.lttng.org
> > Sent: Thursday, November 7, 2013 5:22:35 AM
> > Subject: [lttng-dev] [PATCH lttng-ust 7/7] Add a check against 
> > excluders
> > 
> > When matching enablers with events, first check against all 
> > excluders of the enabler. If the event matches with any of the 
> > excluders, then the event does not match with the enabler.
> > 
> > Signed-off-by: JP Ikaheimonen <jp_ikaheimonen@mentor.com>
> > ---
> >  liblttng-ust/lttng-events.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/liblttng-ust/lttng-events.c 
> > b/liblttng-ust/lttng-events.c index 915cffb..fa5596c 100644
> > --- a/liblttng-ust/lttng-events.c
> > +++ b/liblttng-ust/lttng-events.c
> > @@ -491,6 +491,29 @@ static
> >  int lttng_desc_match_enabler(const struct lttng_event_desc *desc,
> >  		struct lttng_enabler *enabler)
> >  {
> > +	struct lttng_ust_excluder_node *excluder;
> > +
> > +	/* If event matches with an excluder, return 'does not match' */
> > +	cds_list_for_each_entry(excluder, &enabler->excluder_head, node) {
> > +		int found = 0;
> 
> Why is "found" declared here ? Why is it initialized to 0 ?
> 
> 
> > +		int count, len;
> > +		char *excluder_name;
> > +
> > +		for (count = 0; count < excluder->excluder.count; count++) {
> 
> Rather than declared here ?
> 
> 
> > +			excluder_name = (char *)(excluder->excluder.names)
> > +					+ count * LTTNG_UST_SYM_NAME_LEN;
> > +			len = strnlen(excluder_name, LTTNG_UST_SYM_NAME_LEN);
> > +			if (len > 0 && excluder_name[len - 1] == '*') {
> > +				found = strncmp(desc->name, excluder_name, len - 1);
> 
> strncmp and "found" have reversed logic. I'd assign found = !strncmp(....).
> 
> > +			} else {
> > +				found = strncmp(desc->name, excluder_name,
> > +						LTTNG_UST_SYM_NAME_LEN - 1);
> > +			}
> > +			if (!found) {
> 
> and reverse this condition.
> 
> The other patches in this patchset look good !! Good job !!
> 
> Thanks,
> 
> Mathieu
> 
> > +				return 0;
> > +			}
> > +		}
> > +	}
> >  	switch (enabler->type) {
> >  	case LTTNG_ENABLER_WILDCARD:
> >  		return lttng_desc_match_wildcard_enabler(desc, enabler);
> > --
> > 1.8.1.2
> > 
> > 
> > _______________________________________________
> > lttng-dev mailing list
> > lttng-dev@lists.lttng.org
> > http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> > 
> 
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com
> 

--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com