diff mbox series

[lttng-tools,2/3,master,2.10-rc] Fix: lttng list of channels should return errors

Message ID 1495815260-21060-2-git-send-email-mathieu.desnoyers@efficios.com
State Accepted, archived
Delegated to: Jérémie Galarneau
Headers show
Series [lttng-tools,1/3,master,2.10-rc] Fix: discard event/lost packet counters | expand

Commit Message

Mathieu Desnoyers May 26, 2017, 4:14 p.m. UTC
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
---
 src/bin/lttng-sessiond/cmd.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

Comments

Jérémie Galarneau June 12, 2017, 10:23 p.m. UTC | #1
Merged in master, stable-2.10, stable-2.9, and stable-2.8 with a leak
fix, read on.

Thanks!
Jérémie

On 26 May 2017 at 12:14, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
> ---
>  src/bin/lttng-sessiond/cmd.c | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
> index c4e4ccf..dea5ab2 100644
> --- a/src/bin/lttng-sessiond/cmd.c
> +++ b/src/bin/lttng-sessiond/cmd.c
> @@ -243,11 +243,11 @@ end:
>  /*
>   * Fill lttng_channel array of all channels.
>   */
> -static void list_lttng_channels(enum lttng_domain_type domain,
> +static ssize_t list_lttng_channels(enum lttng_domain_type domain,
>                 struct ltt_session *session, struct lttng_channel *channels,
>                 struct lttng_channel_extended *chan_exts)
>  {
> -       int i = 0, ret;
> +       int i = 0, ret = 0;
>         struct ltt_kernel_channel *kchan;
>
>         DBG("Listing channels for session %s", session->name);
> @@ -322,6 +322,9 @@ static void list_lttng_channels(enum lttng_domain_type domain,
>                                 break;
>                         }
>
> +                       chan_exts[i].monitor_timer_interval =
> +                                       uchan->monitor_timer_interval;
> +
>                         ret = get_ust_runtime_stats(session, uchan,
>                                         &discarded_events, &lost_packets);
>                         if (ret < 0) {
> @@ -329,8 +332,6 @@ static void list_lttng_channels(enum lttng_domain_type domain,
>                         }
>                         chan_exts[i].discarded_events = discarded_events;
>                         chan_exts[i].lost_packets = lost_packets;
> -                       chan_exts[i].monitor_timer_interval =
> -                                       uchan->monitor_timer_interval;
>                         i++;
>                 }
>                 rcu_read_unlock();
> @@ -341,7 +342,11 @@ static void list_lttng_channels(enum lttng_domain_type domain,
>         }
>
>  end:
> -       return;
> +       if (ret < 0) {
> +               return -LTTNG_ERR_FATAL;
> +       } else {
> +               return LTTNG_OK;
> +       }
>  }
>
>  static void increment_extended_len(const char *filter_expression,
> @@ -2936,7 +2941,10 @@ ssize_t cmd_list_channels(enum lttng_domain_type domain,
>
>                 channel_exts = ((void *) *channels) +
>                                 (nb_chan * sizeof(struct lttng_channel));
> -               list_lttng_channels(domain, session, *channels, channel_exts);
> +               ret = list_lttng_channels(domain, session, *channels, channel_exts);
> +               if (ret != LTTNG_OK) {

The caller of cmd_list_channels(), process_client_msg(), does not
expect *channels to have been modified on error. Therefore, it is not
free'd in the error path.

I modified the patch so that the "channels" output parameter of
cmd_list_channels() is only affected on success. The function thus
handles the clean-up itself in case of failure.

> +                       goto end;
> +               }
>         } else {
>                 *channels = NULL;
>         }
> --
> 2.1.4
>
diff mbox series

Patch

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index c4e4ccf..dea5ab2 100644
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -243,11 +243,11 @@  end:
 /*
  * Fill lttng_channel array of all channels.
  */
-static void list_lttng_channels(enum lttng_domain_type domain,
+static ssize_t list_lttng_channels(enum lttng_domain_type domain,
 		struct ltt_session *session, struct lttng_channel *channels,
 		struct lttng_channel_extended *chan_exts)
 {
-	int i = 0, ret;
+	int i = 0, ret = 0;
 	struct ltt_kernel_channel *kchan;
 
 	DBG("Listing channels for session %s", session->name);
@@ -322,6 +322,9 @@  static void list_lttng_channels(enum lttng_domain_type domain,
 				break;
 			}
 
+			chan_exts[i].monitor_timer_interval =
+					uchan->monitor_timer_interval;
+
 			ret = get_ust_runtime_stats(session, uchan,
 					&discarded_events, &lost_packets);
 			if (ret < 0) {
@@ -329,8 +332,6 @@  static void list_lttng_channels(enum lttng_domain_type domain,
 			}
 			chan_exts[i].discarded_events = discarded_events;
 			chan_exts[i].lost_packets = lost_packets;
-			chan_exts[i].monitor_timer_interval =
-					uchan->monitor_timer_interval;
 			i++;
 		}
 		rcu_read_unlock();
@@ -341,7 +342,11 @@  static void list_lttng_channels(enum lttng_domain_type domain,
 	}
 
 end:
-	return;
+	if (ret < 0) {
+		return -LTTNG_ERR_FATAL;
+	} else {
+		return LTTNG_OK;
+	}
 }
 
 static void increment_extended_len(const char *filter_expression,
@@ -2936,7 +2941,10 @@  ssize_t cmd_list_channels(enum lttng_domain_type domain,
 
 		channel_exts = ((void *) *channels) +
 				(nb_chan * sizeof(struct lttng_channel));
-		list_lttng_channels(domain, session, *channels, channel_exts);
+		ret = list_lttng_channels(domain, session, *channels, channel_exts);
+		if (ret != LTTNG_OK) {
+			goto end;
+		}
 	} else {
 		*channels = NULL;
 	}