[syslog-ng] [PATCH (3.5)] affile: Error out when there's a syntax error in the filename template

[Balazs Scheidler]

Hi,

Thanks for your patches Gergely, but I would rather see the constructor
to be passed a LogTemplate instance instead of adding an error path to
the constructor. 

This is true to all other patches in the series.

I integrating all your patches from merge-queue but will skip those
affected by this comment.

Cheers,

On h, 2013-06-10 at 16:54 +0200, Gergely Nagy wrote:
> When there is a syntax error in the filename template (eg,
> "/var/log/foo-$(+ 0"), error out at config load time. This is done by
> adding an error output variable to affile_dd_new_instance() and both
> affile_dd_new() and afpipe_dd_new(), then using that to store the result
> of log_template_compile(). If that function fails, we bail out early,
> and use CHECK_ERROR() in the grammar to notify the user of the issue.
> 
> Signed-off-by: Gergely Nagy <algernon at balabit.hu>
> ---
>  modules/affile/affile-dest.c     |   23 +++++++++++++++--------
>  modules/affile/affile-dest.h     |    6 +++---
>  modules/affile/affile-grammar.ym |   12 +++++++++---
>  3 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/modules/affile/affile-dest.c b/modules/affile/affile-dest.c
> index c8fa9c1..c9447ce 100644
> --- a/modules/affile/affile-dest.c
> +++ b/modules/affile/affile-dest.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2002-2012 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2002-2013 BalaBit IT Ltd, Budapest, Hungary
>   * Copyright (c) 1998-2012 Balázs Scheidler
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -703,17 +703,24 @@ affile_dd_free(LogPipe *s)
>  }
>  
>  static AFFileDestDriver *
> -affile_dd_new_instance(gchar *filename)
> +affile_dd_new_instance(gchar *filename, GError **error)
>  {
>    AFFileDestDriver *self = g_new0(AFFileDestDriver, 1);
> +  LogTemplate *filename_template;
> +
> +  filename_template = log_template_new(configuration, NULL);
> +  if (!log_template_compile(filename_template, filename, error))
> +    {
> +      log_template_unref(filename_template);
> +      return NULL;
> +    }
>  
>    log_dest_driver_init_instance(&self->super);
>    self->super.super.super.init = affile_dd_init;
>    self->super.super.super.deinit = affile_dd_deinit;
>    self->super.super.super.queue = affile_dd_queue;
>    self->super.super.super.free_fn = affile_dd_free;
> -  self->filename_template = log_template_new(configuration, NULL);
> -  log_template_compile(self->filename_template, filename, NULL);
> +  self->filename_template = filename_template;
>    log_writer_options_defaults(&self->writer_options);
>    file_perm_options_defaults(&self->file_perm_options);
>    self->writer_options.mark_mode = MM_NONE;
> @@ -728,15 +735,15 @@ affile_dd_new_instance(gchar *filename)
>  }
>  
>  LogDriver *
> -affile_dd_new(gchar *filename)
> +affile_dd_new(gchar *filename, GError **error)
>  {
> -  return &affile_dd_new_instance(filename)->super.super;
> +  return &affile_dd_new_instance(filename, error)->super.super;
>  }
>  
>  LogDriver *
> -afpipe_dd_new(gchar *filename)
> +afpipe_dd_new(gchar *filename, GError **error)
>  {
> -  AFFileDestDriver *self = affile_dd_new_instance(filename);
> +  AFFileDestDriver *self = affile_dd_new_instance(filename, error);
>    self->is_pipe = TRUE;
>    return &self->super.super;
>  }
> diff --git a/modules/affile/affile-dest.h b/modules/affile/affile-dest.h
> index 3f4f21b..dcee654 100644
> --- a/modules/affile/affile-dest.h
> +++ b/modules/affile/affile-dest.h
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2002-2012 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2002-2013 BalaBit IT Ltd, Budapest, Hungary
>   * Copyright (c) 1998-2012 Balázs Scheidler
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -53,8 +53,8 @@ typedef struct _AFFileDestDriver
>    gint time_reap;
>  } AFFileDestDriver;
>  
> -LogDriver *affile_dd_new(gchar *filename);
> -LogDriver *afpipe_dd_new(gchar *filename);
> +LogDriver *affile_dd_new(gchar *filename, GError **error);
> +LogDriver *afpipe_dd_new(gchar *filename, GError **error);
>  
>  void affile_dd_set_create_dirs(LogDriver *s, gboolean create_dirs);
>  void affile_dd_set_fsync(LogDriver *s, gboolean enable);
> diff --git a/modules/affile/affile-grammar.ym b/modules/affile/affile-grammar.ym
> index abecee4..0d2f099 100644
> --- a/modules/affile/affile-grammar.ym
> +++ b/modules/affile/affile-grammar.ym
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2002-2012 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2002-2013 BalaBit IT Ltd, Budapest, Hungary
>   * Copyright (c) 1998-2012 Balázs Scheidler
>   *
>   * This program is free software; you can redistribute it and/or modify it
> @@ -144,7 +144,10 @@ dest_affile
>  dest_affile_params
>  	: string
>  	  {
> -	    last_driver = *instance = affile_dd_new($1);
> +	    GError *error;
> +
> +	    last_driver = *instance = affile_dd_new($1, &error);
> +	    CHECK_ERROR(last_driver, @1, "Error compiling template; %s", error->message);
>  	    free($1);
>  	    last_writer_options = &((AFFileDestDriver *) last_driver)->writer_options;
>  	    last_file_perm_options = &((AFFileDestDriver *) last_driver)->file_perm_options;
> @@ -171,7 +174,10 @@ dest_affile_option
>  dest_afpipe_params
>  	: string
>  	  {
> -	    last_driver = *instance = afpipe_dd_new($1);
> +	    GError *error;
> +
> +	    last_driver = *instance = afpipe_dd_new($1, &error);
> +	    CHECK_ERROR(last_driver, @1, "Error compiling template; %s", error->message);
>  	    free($1);
>  	    last_writer_options = &((AFFileDestDriver *) last_driver)->writer_options;
>  	    last_file_perm_options = &((AFFileDestDriver *) last_driver)->file_perm_options;