[PATCHES v4] Logging to multiple debug backend (was Re: [PATCHES v3] Logging to multiple debug backend)
Amitay Isaacs
amitay at gmail.com
Sun Mar 22 21:13:55 MDT 2015
On Sat, Mar 21, 2015 at 9:54 AM, Christof Schmitt <cs at samba.org> wrote:
> On Fri, Mar 20, 2015 at 07:57:03AM -0700, Christof Schmitt wrote:
> > 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:
> > > > > 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 !
>
> I added a patch at the beginning of the updated patch series to remove
> FORMAT_BUFR_MAX and only use FORMAT_BUFR_SIZE.
>
> > > 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.
>
> Ok, i learned something new about the waf framework. I initially added
> the build checks to the top-level script, since that one has the systemd
> checks. Since systemd is only used in lib/util, those checks can be
> moved to lib/util, and then it also makes sense to add the new checks in
> that directory.
>
Much better, now that systemd/lttng checks are in lib/util.
>
> Here is the updated patch series for review again. I hope that we are
> getting closer :-)
>
Two minor fixes. You can squash them with the appropriate patches.
1. Move the #ifdef outside the conditional. If the condition is false,
there is no code to execute.
2. Since you moved the removal of FORMAT_BUFR_MAX as the first patch,
replace FORMAT_BUFR_MAX with FORMAT_BUFR_SIZE-1
Because of the 1st fix, some of the subsequent patches are slightly
different.
Reviewed-by: me.
Amitay.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patches
Type: application/octet-stream
Size: 72176 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150323/8bd89a5e/attachment.obj>
More information about the samba-technical
mailing list