[syslog-ng]preparing syslog-ng 1.6.6

[Balazs Scheidler]

Hi Roberto,

On Wed, 2005-02-02 at 11:53 +0100, Roberto Nibali wrote:
> Hello,
> 
> [if this email is too confusing, I'll split it up into the 3 subjects]
> 
> > The summary of changes can be found in the file named ChangeLog within 
> > the tarball. If no show-stopper bugs are found, I'm going to release 
> > syslog-ng 1.6.6 this week.
> 
> Ok, the following patch has been applied and works (it's not in the Changelog 
> however), so I can verify this to be fixed:

added the changelog entry.


> Please have a look at following patch which I'd like to submit:
> 
> diff -ur syslog-ng-1.6.5+20050202/src/sources.c syslog-ng-1.6.5+20050202-fixed/s
> rc/sources.c
> --- syslog-ng-1.6.5+20050202/src/sources.c      2004-08-05 13:35:12.000000000 +0
> 200
> +++ syslog-ng-1.6.5+20050202-fixed/src/sources.c        2005-02-02 11:40:04.0000
> 00000 +0100
> @@ -128,6 +128,9 @@
>                               closure->buffer[closure->pos - 1] == '\0'))
>                                  closure->pos--;
>                  }
> +               if (closure->dgram && !eol && closure->pos == closure->max_log_l
> ine) {
> +                       werror("Message length overflow (ignored the rest)\n");
> +               }
>                  do_handle_line(closure, closure->pos, closure->buffer, salen ? (
> abstract_addr *) &sabuf : NULL, salen);
>                  closure->pos = 0;
>                  return ST_OK | ST_GOON;
> 
> The issue here is that we would like to get notified somehow that we've exceeded 
> the max_log_line. This is for tuning reasons and also a reason for our support 
> team not to debug log streams for possible corruption. Please consider applying 
> this patch, it adds only minor computing time to the fast path.

are you sure you want to add the condition as displayed above? that
would mean that only dgram sockets will have this check, however it is
more common to exceed line lengths with stream oriented sockets.

I think you wanted something like

               if (!eol && closure->pos == closure->max_log_line) {
                       werror("Message length overflow (ignored the rest)\n");
               }

Please confirm, then I could add the patch.

> 
> I see that at about the same line the following code has been added:
> 
>                  if (closure->dgram) {
>                          /* strip one trailing LF or NUL character */
>                          if (closure->pos > 0 &&
>                              (closure->buffer[closure->pos - 1] == '\n' ||
>                               closure->buffer[closure->pos - 1] == '\0'))
>                                  closure->pos--;
>                  }
> 
> I've dug out a patch in our CVS which somehow tries to achieve roughly the same 
> but using a different manner (more intrusive). Any comment on this:
> 
> diff -u -r1.37.4.1 sources.c
> --- sources.c   13 Jan 2004 18:08:02 -0000      1.37.4.1
> +++ sources.c   24 Feb 2004 10:37:43 -0000
> @@ -115,7 +115,7 @@
>                  closure->pos = 0;
>                  return ST_OK | ST_GOON;
>          }
> -       if (!eol && (closure->dgram || closure->pos == closure->max_log_line)) {
> +       if (closure->dgram || (!eol && closure->pos == closure->max_log_line)) {
>                  /* we don't have a terminating nl nor \0, and our buffer is
>                     full or we are a datagram receiver, when the message is in
>                     its own packet.
> 
> 
> Our bug report regarding this patchlet can be summarised as follows:
> 
> "syslog-ng assumes that NL or NUL terminates messages regardless of the
> transport medium used."
> 
> Please verify and report back if you have time. It helps us cleaning up our 
> local patch repository :).

I think the same patch is already applied. At least this is what I see
here:

        if (closure->dgram || (!eol && closure->pos == closure->max_log_line)) {
                /* we don't have a terminating nl nor \0, and our buffer is
                   full or we are a datagram receiver, when the message is in
                   its own packet.
                 */
                if (closure->dgram) {
                        /* strip one trailing LF or NUL character */
                        if (closure->pos > 0 &&
                            (closure->buffer[closure->pos - 1] == '\n' ||
                             closure->buffer[closure->pos - 1] == '\0'))
                                closure->pos--;

                }
                do_handle_line(closure, closure->pos, closure->buffer, salen ? (abstract_addr *) &sabuf : NULL, salen);
                closure->pos = 0;
                return ST_OK | ST_GOON;
        }


-- 
Bazsi