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

[Jeremy Allison]

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 !