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@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@balabit.hu> -- Bazsi