[lttng-dev] [PATCH lttng-tools] Fix: destroy session removes the default config file

[Jérémie Galarneau]

Hi Partha,

First, thanks for the patch! Unfortunately, there are a couple of issues
with this fix. Read on.


On Wed, May 20, 2015 at 2:59 PM, Partha Pratim Mukherjee <
ppm.floss at gmail.com> wrote:

> Destroy session command by default removes the default config file
> without checking the current session. As a result when we call any
> other command which expects a default session by calling
> get_session_name() function, it fails.
>
> This patch will fix this by checking that the default config file gets
> removed only when destroy session is called with the current session.
>
> Fixes: #887
>
> Signed-off-by: Partha Pratim Mukherjee <ppm.floss at gmail.com>
> ---
>  src/bin/lttng/commands/destroy.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/bin/lttng/commands/destroy.c
> b/src/bin/lttng/commands/destroy.c
> index 95343c9..3fcecc2 100644
> --- a/src/bin/lttng/commands/destroy.c
> +++ b/src/bin/lttng/commands/destroy.c
> @@ -90,7 +90,9 @@ static int destroy_session(struct lttng_session *session)
>         }
>
>         MSG("Session %s destroyed", session->name);
> -       config_destroy_default();
> +       if (strncmp(session->name, get_session_name(), NAME_MAX) == 0) {
>


This will leak the string returned by get_session_name().

Moreover, config_read_session_name(), which is called by get_session_name()
will spam the error logs anytime an .lttngrc file can't be found; the error
logging statement should be moved out to the different callers.

Looking forward to a v2!
Jérémie



> +               config_destroy_default();
> +       }
>
>         if (lttng_opt_mi) {
>                 ret = mi_lttng_session(writer, session, 0);
> --
> 1.7.10.4
>
>
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>



-- 
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.lttng.org/pipermail/lttng-dev/attachments/20150525/3def2bb1/attachment.html>