[lttng-dev] [PATCH lttng-ust 0/7] UST Event exclusions

[lttng-dev] [PATCH lttng-ust 0/7] UST Event exclusions

[JP Ikaheimonen]

This patch set works in conjunction with the similarly named patch
set for lttng-tools, where an option -x is implemented for the
lttng enable-event command.

In lttng-ust, enablers are enhanced with a list of exclusions - event
names that are not to be matched when matching an enabler with an event
description.

JP Ikaheimonen (7):
  Define a new command LTTNG_UST_EXCLUSION
  Define exclusion structure for UST ABI
  Send exclusion data through ustcomm
  Add excluders to enabler structure
  Store exclusions to enablers
  Add handler for LTTNG_UST_EXCLUSION in UST ABI
  Add a check against excluders

 include/lttng/ust-abi.h       |  8 ++++++
 include/lttng/ust-ctl.h       |  2 ++
 include/lttng/ust-events.h    | 13 +++++++++
 include/ust-comm.h            |  3 +++
 liblttng-ust-ctl/ustctl.c     | 34 +++++++++++++++++++++++
 liblttng-ust/lttng-events.c   | 40 +++++++++++++++++++++++++++
 liblttng-ust/lttng-ust-abi.c  |  7 +++++
 liblttng-ust/lttng-ust-comm.c | 63 +++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 170 insertions(+)

-- 
1.8.1.2

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

[lttng-dev] [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 at 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

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

[Mathieu Desnoyers]

----- Original Message -----
> From: "JP Ikaheimonen" <jp_ikaheimonen@mentor.com>
> To: lttng-dev at 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 at 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 at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

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

[lttng-dev] [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 at mentor.com>
> Cc: lttng-dev at 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 at 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 at 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 at 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 at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

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

[lttng-dev] [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 at 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 at 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 at 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 at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> 

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

[lttng-dev] [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 at efficios.com>
> Cc: lttng-dev at 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 at 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 at 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 at 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 at 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

[lttng-dev] [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 at 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 at efficios.com>
> Cc: lttng-dev at 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 at 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 at 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 at 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 at 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