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

[Amitay Isaacs]

On Fri, Mar 20, 2015 at 9:11 AM, Jeremy Allison <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.

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.

Amitay.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SQ-Fix-lttng-ust-dependency-for-standalone-ctdb-buil.patch
Type: text/x-patch
Size: 749 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150320/d0eca345/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-SQ-Fix-systemd-journal-depedency-for-standalone-ctdb.patch
Type: text/x-patch
Size: 756 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150320/d0eca345/attachment-0001.bin>