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

Jeremy Allison jra at samba.org
Thu Mar 19 16:11:09 MDT 2015


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 !


More information about the samba-technical mailing list