[PATCHES v3] Logging to multiple debug backend (was Re: [PATCHES v2] Logging to multiple debug backends)

[Christof Schmitt]

On Fri, Mar 20, 2015 at 12:39:48PM +1100, Amitay Isaacs wrote:
>    On Fri, Mar 20, 2015 at 9:11 AM, Jeremy Allison <[1]jra at samba.org> wrote:
> 
>      On Thu, Mar 19, 2015 at 02:58:00PM -0700, Christof Schmitt wrote:
>      > On Thu, Mar 19, 2015 at 01:44:51PM -0700, Jeremy Allison wrote:
>      > > On Thu, Mar 19, 2015 at 01:20:37PM -0700, Christof Schmitt wrote:
>      > > > > +       char msg_no_nl[FORMAT_BUFR_SIZE];
>      > > > > +       int len = strlen(msg);
>      > > > > +       int i;
>      > > > > +
>      > > > > +       /*
>      > > > > +        * Some backends already add an extra newline, so also
>      provide
>      > > > > +        * a buffer without the newline character.
>      > > > > +        */
>      > > > > +       if (msg[len - 1] == '\n') {
>      > > > > +               len--;
>      > > > > +       }
>      > > > > +
>      > > > > +       memcpy(msg_no_nl, msg, len);
>      > > > > +       msg_no_nl[len] = '\0';
>      > > > >
>      > > > > the memcpy length needs to be clamped to the
>      > > > > max of FORMAT_BUFR_SIZE-1, Otherwise if the
>      > > > > lenth of *msg > FORMAT_BUFR_SIZE we're in
>      > > > > trouble :-).
>      > > > >
>      > > > > Other than that it looks very nice work !
>      > > > >
>      > > > > Can you fix that up and resubmit and I
>      > > > > promise I'll re-review.
>      > > >
>      > > > I think that this cannot happen since the callers of
>      > > > Debug1->debug_backends_log do not send messages larger than
>      > > > FORMAT_BUFR_SIZE, but is probably better to be safe.
>      > >
>      > > Wow. I *really* hate the:
>      > >
>      > > #define FORMAT_BUFR_SIZE 1024
>      > > #define FORMAT_BUFR_MAX (FORMAT_BUFR_SIZE - 1)
>      > >
>      > >
>      > > +       char msg_no_nl[FORMAT_BUFR_SIZE];
>      > > ...
>      > > +       len = MIN(strlen(msg), FORMAT_BUFR_MAX);
>      > >
>      > > idiom where a #define length is used as an array
>      > > specifier, and a *different* #define is used
>      > > as a length limiter.... But that's a patch
>      > > for another day :-).
>      > >
>      > > Other than that - LGTM. Pushed, thanks !
>      >
>      > Looks like i overlooked the stand-alone ctdb build, and the autobuild
>      > failed due to unknown dependencies in the samba-debug code while
>      > building ctdb. I am resolving this now.
> 
>      Yep, discovered that :-). Thanks for resolving !
> 
>    Hi Christof,
> 
>    Here are the fixes to get standalone ctdb build working.  You can squash
>    them with the patches that introduce systemd and lttng backends.

Thank you.

>    However, why are the options for systemd and lttng added to top-level
>    wscript?  Since the systemd and lttng features are only required for
>    logging, those options could be added in lib/util/wscript instead of
>    top-level wscript.  In that case, there won't be a need for special fix
>    for standalone ctdb.

Good point. I was probably thinking more in terms of the old configure
script than the new waf framework. Let me try to move the checks.

Christof

> 
>    Amitay.
> 
> References
> 
>    Visible links
>    1. mailto:jra at samba.org

> From abe9416eb3eaa74fcecf87372562734606a29391 Mon Sep 17 00:00:00 2001
> From: Amitay Isaacs <amitay at gmail.com>
> Date: Fri, 20 Mar 2015 12:31:12 +1100
> Subject: [PATCH] SQ: Fix lttng-ust dependency for standalone ctdb build
> 
> Signed-off-by: Amitay Isaacs <amitay at gmail.com>
> ---
>  ctdb/wscript | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ctdb/wscript b/ctdb/wscript
> index f6ac7a6..b5defea 100755
> --- a/ctdb/wscript
> +++ b/ctdb/wscript
> @@ -214,6 +214,7 @@ def configure(conf):
>  
>          conf.SET_TARGET_TYPE('systemd-daemon', 'EMPTY')
>          conf.SET_TARGET_TYPE('systemd-journal', 'EMPTY')
> +        conf.SET_TARGET_TYPE('lttng-ust', 'EMPTY')
>  
>          del(conf.env.defines['PYTHONDIR'])
>          del(conf.env.defines['PYTHONARCHDIR'])
> -- 
> 2.1.0
> 

> From 3a8f79ab21fc841ef348e9d5d099d67ecf227328 Mon Sep 17 00:00:00 2001
> From: Amitay Isaacs <amitay at gmail.com>
> Date: Fri, 20 Mar 2015 12:28:57 +1100
> Subject: [PATCH] SQ: Fix systemd-journal depedency for standalone ctdb build
> 
> Signed-off-by: Amitay Isaacs <amitay at gmail.com>
> ---
>  ctdb/wscript | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ctdb/wscript b/ctdb/wscript
> index 5957f16..f6ac7a6 100755
> --- a/ctdb/wscript
> +++ b/ctdb/wscript
> @@ -213,6 +213,7 @@ def configure(conf):
>          conf.ADD_EXTRA_INCLUDES('#lib #lib/replace')
>  
>          conf.SET_TARGET_TYPE('systemd-daemon', 'EMPTY')
> +        conf.SET_TARGET_TYPE('systemd-journal', 'EMPTY')
>  
>          del(conf.env.defines['PYTHONDIR'])
>          del(conf.env.defines['PYTHONARCHDIR'])
> -- 
> 2.1.0
>