[syslog-ng] [PATCH]: support setting a default suppress() globally

Balazs Scheidler bazsi at balabit.hu
Tue Mar 1 21:49:09 CET 2011


Hi Gergely,

Sorry for not reviewing your patch any sooner, but you know the
drill. :)

Could you please post your patches in-line next time? It is much easier
to read and react on things that way. Thanks.

But here comes the review:


> diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym
> index 2a7c134..5379b81 100644
> --- a/modules/affile/affile-grammar.ym
> +++ b/modules/affile/affile-grammar.ym
> @@ -132,6 +132,7 @@ dest_affile_params
>             (*last_driver) = affile_dd_new($1, 0);
>             free($1);
>             last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options;
> +           last_writer_options->suppress = configuration->suppress;
>           }
>           dest_affile_options                           { $$ = (*last_driver); }
>         ;

A better way to do this would be to do the inheritance in the 
log_writer_options_init() function, just like all the rest of the options.

E.g. please initialize the per-writer suppress to default to -1 (in 
log_writer_options_defaults), and in log_writer_options_init() inherit the
global value if the setting stays at -1.

I guess all the rest should be implemented this way.

I really like the patch, so please post an updated patch for inclusion.

Thanks.

> @@ -163,6 +164,7 @@ dest_afpipe_params
>             free($1);
>             last_writer_options = &((AFFileDestDriver *) (*last_driver))->writer_options;
>             last_writer_options->flush_lines = 0;
> +           last_writer_options->suppress = configuration->suppress;
>           }
>           dest_afpipe_options                   { $$ = (*last_driver); }
>         ;

hmm.. that flush_lines initialization seems spiffy, so I've removed 
that with this patch:

commit ac51af8f501ed5b723c055cbb30c0cb1e09f0117
Author: Balazs Scheidler <bazsi at balabit.hu>
Date:   Tue Mar 1 21:47:15 2011 +0100

    pipe(): use the default flush_lines() value if one is not specified
    
    For some historic reason the pipe() destination overrode the global
    flush_lines() settings by default. This is inconsistent behaviour, so fix
    it.
    
    Signed-off-by: Balazs Scheidler <bazsi at balabit.hu>



-- 
Bazsi



More information about the syslog-ng mailing list