[syslog-ng] [PATCH 2/4] value-pairs: Type hinting support
Balazs Scheidler
bazsi at balabit.hu
Sun Feb 17 13:49:24 CET 2013
Hi,
Thanks for the great contribution, I feel this is another leap for
syslog-ng's flexibility. :)
This is a big one, which I didn't yet integrate, see my comments inline.
On Fri, 2013-02-15 at 16:26 +0100, Gergely Nagy wrote:
> This implements very basic support for type hinting template strings,
> where those hints will be available to value-pairs too, allowing any
> user of either LogTemplate or value-pairs to handle different types of
> values than plain strings. How they handle it, is up to the callers,
> this patch just lays the groundwork that makes it possible to pass
> these hints around.
>
> Furthermore, this includes support for the on-error() statement, which
> tells drivers how type-casting failures shall be handled: they can
> either drop the message, drop the property, or fall back to string;
> and can do either of these either verbosely, logging the fact, or
> silently.
>
> This fixes #4 and #5.
>
> Signed-off-by: Gergely Nagy <algernon at balabit.hu>
> ---
> lib/Makefile.am | 2 +
> lib/cfg-grammar.y | 86 ++++++++++++++--
> lib/cfg-parser.c | 4 +
> lib/cfg.c | 2 +
> lib/cfg.h | 2 +
> lib/driver.c | 7 ++
> lib/driver.h | 2 +
> lib/scratch-buffers.c | 48 +++++++++
> lib/scratch-buffers.h | 17 ++++
> lib/templates.c | 1 +
> lib/templates.h | 3 +
> lib/type-hinting.c | 226 +++++++++++++++++++++++++++++++++++++++++
> lib/type-hinting.h | 73 +++++++++++++
> lib/value-pairs.c | 153 +++++++++++++++++++++-------
> lib/value-pairs.h | 36 ++++---
> modules/afamqp/afamqp.c | 3 +-
> modules/afmongodb/afmongodb.c | 2 +-
> modules/json/format-json.c | 3 +-
> tests/unit/Makefile.am | 1 +
> tests/unit/test_type_hints.c | 191 ++++++++++++++++++++++++++++++++++
> tests/unit/test_value_pairs.c | 8 +-
> 21 files changed, 801 insertions(+), 69 deletions(-)
> create mode 100644 lib/type-hinting.c
> create mode 100644 lib/type-hinting.h
> create mode 100644 tests/unit/test_type_hints.c
>
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 8780394..d4b50cc 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -83,6 +83,7 @@ pkginclude_HEADERS = \
> tls-support.h \
> tlscontext.h \
> tlstransport.h \
> + type-hinting.h \
> utils.h \
> uuid.h \
> value-pairs.h \
> @@ -161,6 +162,7 @@ libsyslog_ng_la_SOURCES = \
> tags.c \
> templates.c \
> timeutils.c \
> + type-hinting.c \
> utils.c \
> value-pairs.c \
> vptransform.c \
> diff --git a/lib/cfg-grammar.y b/lib/cfg-grammar.y
> index f45d486..c48b4cb 100644
> --- a/lib/cfg-grammar.y
> +++ b/lib/cfg-grammar.y
> @@ -32,6 +32,7 @@
> /* YYSTYPE and YYLTYPE is defined by the lexer */
> #include "cfg-lexer.h"
> #include "afinter.h"
> +#include "type-hinting.h"
> #include "filter-expr-parser.h"
> #include "parser-expr-parser.h"
> #include "rewrite-expr-parser.h"
> @@ -49,6 +50,7 @@ extern struct _FilePermOptions *last_file_perm_options;
> extern struct _MsgFormatOptions *last_msg_format_options;
> extern struct _LogDriver *last_driver;
> extern struct _LogParser *last_parser;
> +LogTemplate *last_template;
I would like to see this variable gone, or at least narrow its scope to
be used only by the top-level template {} statement. By returning the
template object from template_content we can eliminate most of it.
>
> }
>
> @@ -303,6 +305,9 @@ extern struct _LogParser *last_parser;
> %token KW_ADD_PREFIX 10508
> %token KW_REPLACE 10509
>
> +%token KW_TYPE_CAST 10510
> +%token KW_ON_ERROR 10511
> +
> /* END_DECLS */
>
> %code {
> @@ -341,7 +346,6 @@ LogReaderOptions *last_reader_options;
> LogWriterOptions *last_writer_options;
> MsgFormatOptions *last_msg_format_options;
> FilePermOptions *last_file_perm_options;
> -LogTemplate *last_template;
> CfgArgs *last_block_args;
> ValuePairs *last_value_pairs;
> ValuePairsTransformSet *last_vp_transset;
> @@ -370,6 +374,8 @@ ValuePairsTransformSet *last_vp_transset;
> %type <ptr> dest_item
> %type <ptr> dest_plugin
>
> +%type <ptr> template_content
> +
> %type <ptr> filter_content
>
> %type <ptr> parser_content
> @@ -396,6 +402,7 @@ ValuePairsTransformSet *last_vp_transset;
> /* START_DECLS */
>
> %type <ptr> value_pair_option
> +%type <num> on_error_stmt
>
> %type <num> yesno
> %type <num> dnsmode
> @@ -709,13 +716,37 @@ template_items
> |
> ;
>
> -template_item
> - : KW_TEMPLATE '(' string ')' {
> - GError *error = NULL;
> +/* START_RULES */
>
> - CHECK_ERROR(log_template_compile(last_template, $3, &error), @3, "Error compiling template (%s)", error->message);
> - free($3);
> - }
> +template_content
> + : string
> + {
> + GError *error = NULL;
> +
> + CHECK_ERROR(log_template_compile(last_template, $1, &error), @1, "Error compiling template (%s)", error->message);
> + free($1);
> + }
> + | LL_IDENTIFIER '(' string ')'
> + {
> + GError *error = NULL;
> + TypeHint type;
> +
> + if (!last_template)
> + last_template = log_template_new(configuration, NULL);
This is not nice, the rule above this simply assumes that last_template
is set, while this is not. We probably can't completely merge the
parsing rules for template_stmt with all the other places.
I'd say that template_content should always create a new template and
return it in $$, template_stmt can then use an independent
implementation.
> +
> + CHECK_ERROR(log_template_compile(last_template, $3, &error), @3, "Error compiling template (%s)", error->message);
> + free($3);
> +
> + CHECK_ERROR(type_hint_parse($1, &type, &error), @1, "Error setting the template type-hint (%s)", error->message);
> + last_template->type_hint = type;
this parsing should be delegated to LogTemplate and not poked into the
structure.
> + free($1);
> + }
> + ;
> +
> +/* END_RULES */
> +
> +template_item
> + : KW_TEMPLATE '(' template_content ')'
> | KW_TEMPLATE_ESCAPE '(' yesno ')' { log_template_set_escape(last_template, $3); }
> ;
>
> @@ -762,6 +793,24 @@ block_arg
> }
> ;
>
> +/* START_RULES */
> +
> +on_error_stmt
> + : KW_ON_ERROR '(' string ')'
> + {
> + GError *error = NULL;
> + gint on_error;
> +
> + CHECK_ERROR(type_cast_strictness_parse($3, &on_error, &error),
> + @3, "Invalid strictness");
> + free($3);
> +
> + $$ = on_error;
> + }
> + ;
> +
> +/* END_RULES */
> +
> options_items
> : options_item ';' options_items { $$ = $1; }
> | { $$ = NULL; }
> @@ -822,6 +871,7 @@ options_item
> | KW_RECV_TIME_ZONE '(' string ')' { configuration->recv_time_zone = g_strdup($3); free($3); }
> | KW_SEND_TIME_ZONE '(' string ')' { configuration->template_options.time_zone[LTZ_SEND] = g_strdup($3); free($3); }
> | KW_LOCAL_TIME_ZONE '(' string ')' { configuration->template_options.time_zone[LTZ_LOCAL] = g_strdup($3); free($3); }
> + | KW_TYPE_CAST '(' on_error_stmt ')' { configuration->type_cast_strictness = $3; }
> ;
Do you have any other idea what we might add into the type-cast option?
Do we really need a new block for that?
If I understand correctly this looks like:
options { type-cast(on-error(xxxx)); };
I'd prefer type-cast-error-action(xxx) as a top-level action, unless
there's a strong possibility that we will need another layer of nesting
in the future.
I'd perhaps also ask the mailing list (and the documentation guys),
which form they'd prefer.
>
> /* START_RULES */
> @@ -1065,8 +1115,26 @@ vp_options
> ;
>
> vp_option
> - : KW_PAIR '(' string ':' string ')' { value_pairs_add_pair(last_value_pairs, configuration, $3, $5); free($3); free($5); }
> - | KW_PAIR '(' string string ')' { value_pairs_add_pair(last_value_pairs, configuration, $3, $4); free($3); free($4); }
> + : KW_PAIR '(' string ':' template_content ')'
> + {
> + GError *error = NULL;
> +
> + CHECK_ERROR(value_pairs_add_pair(last_value_pairs, configuration, $3,
> + last_template->type_hint,
> + last_template->template, &error),
> + @5, "Error processing value-pair (%s)", error->message);
> + free($3);
> + }
if $5 would be last_template, we would not have to use a global variable
here.
> + | KW_PAIR '(' string template_content ')'
> + {
> + GError *error = NULL;
> +
> + CHECK_ERROR(value_pairs_add_pair(last_value_pairs, configuration, $3,
> + last_template->type_hint,
> + last_template->template, &error),
> + @4, "Error processing value-pair (%s)", error->message);
> + free($3);
> + }
likewise.
Also, I don't like the fact that value-pairs need an explicit type_hint
here, why not ask from the template instance there?
Ah, I think I understand, value-pairs do their own management of
LogTemplate instances, which means we have a leak here.
Why not change add_pair() to expect a LogTemplate pointer instead?
> | KW_KEY '(' string KW_REKEY '('
> {
> last_vp_transset = value_pairs_transform_set_new($3);
> diff --git a/lib/cfg-parser.c b/lib/cfg-parser.c
> index 222a8cf..538dd2d 100644
> --- a/lib/cfg-parser.c
> +++ b/lib/cfg-parser.c
> @@ -67,6 +67,10 @@ static CfgLexerKeyword main_keywords[] = {
> { "add_prefix", KW_ADD_PREFIX, 0x0303 },
> { "replace", KW_REPLACE, 0x0303 },
>
> + /* type hints */
> + { "typecast", KW_TYPE_CAST, 0x0304 },
the convention is to use dashes where underscores are present in the
KW_XXX value.
So this should be type-cast or rather type_cast as the lexer makes _ and
dash equivalent in keywords.
> + { "on_error", KW_ON_ERROR, 0x0304 },
> +
> /* option items */
> { "flags", KW_FLAGS },
> { "pad_size", KW_PAD_SIZE },
> diff --git a/lib/cfg.c b/lib/cfg.c
> index 610c448..c305c04 100644
> --- a/lib/cfg.c
> +++ b/lib/cfg.c
> @@ -325,6 +325,8 @@ cfg_new(gint version)
> self->recv_time_zone = NULL;
> self->keep_timestamp = TRUE;
>
> + self->type_cast_strictness = TYPE_CAST_DROP_MESSAGE;
> +
> cfg_tree_init_instance(&self->tree, self);
> return self;
> }
> diff --git a/lib/cfg.h b/lib/cfg.h
> index 86b9a9a..db70ff9 100644
> --- a/lib/cfg.h
> +++ b/lib/cfg.h
> @@ -31,6 +31,7 @@
> #include "cfg-parser.h"
> #include "persist-state.h"
> #include "templates.h"
> +#include "type-hinting.h"
>
> #include <sys/types.h>
> #include <regex.h>
> @@ -84,6 +85,7 @@ struct _GlobalConfig
> gint time_reopen;
> gint time_reap;
> gint suppress;
> + gint type_cast_strictness;
>
> gint log_fifo_size;
> gint log_msg_size;
> diff --git a/lib/driver.c b/lib/driver.c
> index d77fe57..0e2b4e4 100644
> --- a/lib/driver.c
> +++ b/lib/driver.c
> @@ -282,6 +282,8 @@ log_dest_driver_deinit_method(LogPipe *s)
> void
> log_dest_driver_init_instance(LogDestDriver *self)
> {
> + GlobalConfig *config;
> +
> log_driver_init_instance(&self->super);
> self->super.super.init = log_dest_driver_init_method;
> self->super.super.deinit = log_dest_driver_deinit_method;
> @@ -290,6 +292,11 @@ log_dest_driver_init_instance(LogDestDriver *self)
> self->release_queue = log_dest_driver_release_queue_method;
> self->log_fifo_size = -1;
> self->throttle = 0;
> +
> + config = log_pipe_get_config((LogPipe *)self);
> + if (!config)
> + config = configuration;
this is wrong, this should probably be moved into the init() method
(this init_instance is the constructor, I know the naming is confusing
sometimes).
log_pipe_get_config() should always be set there. It's a bug elsewhere
if it is not.
The global configuration variable should ever be used while syslog-ng is
running the parser code.
The usual pattern for variables like this is to set them to an extremal
value that indicates the global value should be used, and then fetch
that value in the _init() method
> + self->type_cast_strictness = config->type_cast_strictness;
> }
>
> void
> diff --git a/lib/driver.h b/lib/driver.h
> index 4e0bf47..c0ad669 100644
> --- a/lib/driver.h
> +++ b/lib/driver.h
> @@ -29,6 +29,7 @@
> #include "logpipe.h"
> #include "logqueue.h"
> #include "cfg.h"
> +#include "type-hinting.h"
>
> /*
> * Drivers overview
> @@ -161,6 +162,7 @@ struct _LogDestDriver
> gint log_fifo_size;
> gint throttle;
> StatsCounterItem *queued_global_messages;
> + gint type_cast_strictness;
> };
>
> /* returns a reference */
> diff --git a/lib/scratch-buffers.c b/lib/scratch-buffers.c
> index 481ce73..7c21e04 100644
> --- a/lib/scratch-buffers.c
> +++ b/lib/scratch-buffers.c
> @@ -29,6 +29,7 @@
> TLS_BLOCK_START
> {
> GTrashStack *sb_gstrings;
> + GTrashStack *sb_th_gstrings;
> GList *sb_registry;
> }
> TLS_BLOCK_END;
> @@ -80,6 +81,52 @@ ScratchBufferStack SBGStringStack = {
> .free_stack = sb_gstring_free_stack
> };
>
> +/* Type-hinted GStrings */
> +
> +#define local_sb_th_gstrings __tls_deref(sb_th_gstrings)
> +
> +GTrashStack *
> +sb_th_gstring_acquire_buffer (void)
> +{
> + SBTHGString *sb;
> +
> + sb = g_trash_stack_pop(&local_sb_th_gstrings);
> + if (!sb)
> + {
> + sb = g_new(SBTHGString, 1);
> + g_string_steal(sb_th_gstring_string(sb));
> + sb->type_hint = TYPE_HINT_STRING;
> + }
> + else
> + g_string_set_size(sb_th_gstring_string(sb), 0);
> +
> + return (GTrashStack *)sb;
> +}
> +
> +void
> +sb_th_gstring_release_buffer(GTrashStack *s)
> +{
> + g_trash_stack_push(&local_sb_th_gstrings, s);
> +}
> +
> +void
> +sb_th_gstring_free_stack(void)
> +{
> + SBGString *sb;
> +
> + while ((sb = g_trash_stack_pop(&local_sb_th_gstrings)) != NULL)
> + {
> + g_free(sb_gstring_string(sb)->str);
> + g_free(sb);
> + }
> +}
> +
> +ScratchBufferStack SBTHGStringStack = {
> + .acquire_buffer = sb_th_gstring_acquire_buffer,
> + .release_buffer = sb_th_gstring_release_buffer,
> + .free_stack = sb_th_gstring_free_stack
> +};
> +
> /* Global API */
>
> #define local_sb_registry __tls_deref(sb_registry)
> @@ -95,6 +142,7 @@ scratch_buffers_init(void)
> {
> local_sb_registry = NULL;
> scratch_buffers_register(&SBGStringStack);
> + scratch_buffers_register(&SBTHGStringStack);
> }
>
> static void
Well, I start to dislike the scratch-buffer API, if we have to add
subsystem specific code to the scratch-buffer implementation in order to
add new structures is not nice.
Perhaps this could be moved into the type-hinting code?
Also, I don't really like that this seems like a general data structure
(type-hint + string), whereas this is a very specific implementation
dictated structure used by the value-pairs code.
I think I now understand why started shoving the type-hint value into
the GString somewhere earlier.
The value-pairs callback should probably get an opaque const VPValue
structure, containing all the things needed to resolve the actual value
of the value-pair.
I'll try to write more about the idea in the value-pairs code (hopefully
somewhat below).
> diff --git a/lib/scratch-buffers.h b/lib/scratch-buffers.h
> index 72ab40a..66c701f 100644
> --- a/lib/scratch-buffers.h
> +++ b/lib/scratch-buffers.h
> @@ -26,6 +26,7 @@
> #define SCRATCH_BUFFERS_H_INCLUDED 1
>
> #include <glib.h>
> +#include "type-hinting.h"
>
> /* Global API */
>
> @@ -67,4 +68,20 @@ extern ScratchBufferStack SBGStringStack;
>
> #define sb_gstring_string(buffer) (&buffer->s)
>
> +/* Type-hinted GStrings */
> +
> +typedef struct
> +{
> + GTrashStack stackp;
> + GString s;
> + TypeHint type_hint;
> +} SBTHGString;
> +
> +extern ScratchBufferStack SBTHGStringStack;
> +
> +#define sb_th_gstring_acquire() ((SBTHGString *)scratch_buffer_acquire(&SBTHGStringStack))
> +#define sb_th_gstring_release(b) (scratch_buffer_release(&SBTHGStringStack, (GTrashStack *)b))
> +
> +#define sb_th_gstring_string(buffer) (&buffer->s)
> +
> #endif
> diff --git a/lib/templates.c b/lib/templates.c
> index f62f8b8..fb312e9 100644
> --- a/lib/templates.c
> +++ b/lib/templates.c
> @@ -1273,6 +1273,7 @@ log_template_new(GlobalConfig *cfg, gchar *name)
> LogTemplate *self = g_new0(LogTemplate, 1);
>
> self->name = g_strdup(name);
> + self->type_cast_strictness = TYPE_CAST_DROP_MESSAGE;
hmm.. isn't this a destinstation driver property? where does it get set?
> self->ref_cnt = 1;
> self->cfg = cfg;
> g_static_mutex_init(&self->arg_lock);
> diff --git a/lib/templates.h b/lib/templates.h
> index ed5b688..69f005b 100644
> --- a/lib/templates.h
> +++ b/lib/templates.h
> @@ -27,6 +27,7 @@
>
> #include "syslog-ng.h"
> #include "timeutils.h"
> +#include "type-hinting.h"
>
> #define LTZ_LOCAL 0
> #define LTZ_SEND 1
> @@ -54,6 +55,8 @@ typedef struct _LogTemplate
> GlobalConfig *cfg;
> GStaticMutex arg_lock;
> GPtrArray *arg_bufs;
> + TypeHint type_hint;
> + gint type_cast_strictness;
> } LogTemplate;
>
> /* template expansion options that can be influenced by the user and
> diff --git a/lib/type-hinting.c b/lib/type-hinting.c
> new file mode 100644
> index 0000000..7be91a1
> --- /dev/null
> +++ b/lib/type-hinting.c
> @@ -0,0 +1,226 @@
> +/*
> + * Copyright (c) 2012-2013 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2012-2013 Gergely Nagy <algernon at balabit.hu>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + * As an additional exemption you are allowed to compile & link against the
> + * OpenSSL libraries as published by the OpenSSL project. See the file
> + * COPYING for details.
> + *
> + */
> +
> +#include "messages.h"
> +#include "type-hinting.h"
> +#include "templates.h"
> +
> +#include <string.h>
> +#include <stdlib.h>
> +
> +GQuark
> +type_hinting_error_quark()
> +{
> + return g_quark_from_static_string("type-hinting-error-quark");
> +}
> +
> +gboolean
> +type_hint_parse(const gchar *hint, TypeHint *out_type, GError **error)
> +{
> + if (hint == NULL)
> + {
> + *out_type = TYPE_HINT_STRING;
> + return TRUE;
> + }
> +
> + if (strcmp(hint, "string") == 0)
> + *out_type = TYPE_HINT_STRING;
> + else if (strcmp(hint, "literal") == 0)
> + *out_type = TYPE_HINT_LITERAL;
> + else if (strcmp(hint, "int32") == 0 || strcmp(hint, "int") == 0)
> + *out_type = TYPE_HINT_INT32;
> + else if (strcmp(hint, "int64") == 0)
> + *out_type = TYPE_HINT_INT64;
> + else if (strcmp(hint, "datetime") == 0)
> + *out_type = TYPE_HINT_DATETIME;
> + else if (strcmp(hint, "boolean") == 0)
> + *out_type = TYPE_HINT_BOOLEAN;
> + else if (strcmp(hint, "default") == 0)
> + *out_type = TYPE_HINT_DEFAULT;
> + else
> + {
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_TYPE,
> + "%s", hint);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_strictness_parse(const gchar *strictness, gint *out, GError **error)
> +{
> + const gchar *p = strictness;
> + gboolean silently = FALSE;
> +
> + if (!strictness)
> + {
> + *out = TYPE_CAST_DROP_MESSAGE;
> + return TRUE;
> + }
> +
> + if (strncmp(strictness, "silently-", strlen("silently-")) == 0)
> + {
> + silently = TRUE;
> + p = strictness + strlen("silently-");
> + }
> +
> + if (strcmp(p, "drop-message") == 0)
> + *out = TYPE_CAST_DROP_MESSAGE;
> + else if (strcmp(p, "drop-property") == 0)
> + *out = TYPE_CAST_DROP_PROPERTY;
> + else if (strcmp(p, "fallback-to-string") == 0)
> + *out = TYPE_CAST_FALLBACK_TO_STRING;
> + else
> + {
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_CAST_INVALID_STRICTNESS,
> + "%s",strictness);
> + return FALSE;
> + }
> +
> + if (silently)
> + *out |= TYPE_CAST_SILENTLY;
> +
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_drop_helper(gint drop_flags, const gchar *value,
> + const gchar *type_hint)
> +{
> + if (!(drop_flags & TYPE_CAST_SILENTLY))
> + {
> + msg_error("Casting error",
> + evt_tag_str("value", value),
> + evt_tag_str("type-hint", type_hint),
> + NULL);
> + }
> + return drop_flags & TYPE_CAST_DROP_MESSAGE;
> +}
> +
> +gboolean
> +type_cast_to_boolean(const gchar *value, gboolean *out, GError **error)
> +{
> + if (value[0] == 'T' || value[0] == 't' || value[0] == '1')
> + *out = TRUE;
> + else if (value[0] == 'F' || value[0] == 'f' || value[0] == '0')
> + *out = FALSE;
> + else
> + {
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "boolean(%s)", value);
> + return FALSE;
> + }
> +
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_to_int32(const gchar *value, gint32 *out, GError **error)
> +{
> + gchar *endptr;
> +
> + *out = (gint32)strtol(value, &endptr, 10);
> +
> + if (endptr[0] != '\0')
> + {
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "int32(%s)", value);
> + return FALSE;
> + }
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_to_int64(const gchar *value, gint64 *out, GError **error)
> +{
> + gchar *endptr;
> +
> + *out = (gint64)strtoll(value, &endptr, 10);
> +
> + if (endptr[0] != '\0')
> + {
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "int64(%s)", value);
> + return FALSE;
> + }
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_to_datetime_int(const gchar *value, guint64 *out, GError **error)
> +{
> + gchar *endptr;
> +
> + *out = (gint64)strtoll(value, &endptr, 10) * 1000;
> +
> + if (endptr[0] == '.')
> + {
> + gsize len = strlen(endptr) - 1, p;
> + gchar *e, tmp[4];
> + glong i;
> +
> + if (len > 3)
> + len = 3;
> +
> + memcpy(tmp, endptr + 1, len);
> + tmp[len] = '\0';
> +
> + i = strtoll(tmp, &e, 10);
> +
> + if (e[0] != '\0')
> + {
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "datetime(%s)", value);
> + return FALSE;
> + }
> +
> + for (p = 3 - len; p > 0; p--)
> + i *= 10;
> +
> + *out += i;
> + }
> + else if (endptr[0] != '\0')
> + {
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "datetime(%s)", value);
> + return FALSE;
> + }
> + return TRUE;
> +}
> +
> +gboolean
> +type_cast_to_datetime_str(const gchar *value, const char *format,
> + gchar **out, GError **error)
> +{
> + if (error)
> + g_set_error(error, TYPE_HINTING_ERROR, TYPE_HINTING_INVALID_CAST,
> + "datetime_str is not supported yet");
> + return FALSE;
> +}
> diff --git a/lib/type-hinting.h b/lib/type-hinting.h
> new file mode 100644
> index 0000000..94ec757
> --- /dev/null
> +++ b/lib/type-hinting.h
> @@ -0,0 +1,73 @@
> +/*
> + * Copyright (c) 2012-2013 BalaBit IT Ltd, Budapest, Hungary
> + * Copyright (c) 2012-2013 Gergely Nagy <algernon at balabit.hu>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + * As an additional exemption you are allowed to compile & link against the
> + * OpenSSL libraries as published by the OpenSSL project. See the file
> + * COPYING for details.
> + *
> + */
> +
> +#ifndef TYPE_HINTING_H
> +#define TYPE_HINTING_H
> +
> +#include "syslog-ng.h"
> +
> +#define TYPE_HINTING_ERROR type_hinting_error_quark()
> +
> +GQuark type_hinting_error_quark(void);
> +
> +enum TypeHintingError
> +{
> + TYPE_HINTING_INVALID_TYPE,
> + TYPE_HINTING_INVALID_CAST,
> + TYPE_CAST_INVALID_STRICTNESS,
> +};
> +
> +typedef enum
> +{
> + TYPE_HINT_STRING,
> + TYPE_HINT_LITERAL,
> + TYPE_HINT_BOOLEAN,
> + TYPE_HINT_INT32,
> + TYPE_HINT_INT64,
> + TYPE_HINT_DATETIME,
> + TYPE_HINT_DEFAULT,
> +} TypeHint;
> +
> +typedef enum
> +{
> + TYPE_CAST_DROP_MESSAGE = 0x01,
> + TYPE_CAST_DROP_PROPERTY = 0x02,
> + TYPE_CAST_FALLBACK_TO_STRING = 0x04,
> + TYPE_CAST_SILENTLY = 0x08
> +} TypeCastStrictness;
> +
> +gboolean type_cast_strictness_parse(const gchar *strictness, gint *out, GError **error);
> +gboolean type_hint_parse(const gchar *hint, TypeHint *out_hint, GError **error);
> +
> +gboolean type_cast_drop_helper(gint drop_flags, const gchar *value,
> + const gchar *type_hint);
> +
> +gboolean type_cast_to_boolean(const gchar *value, gboolean *out, GError **error);
> +gboolean type_cast_to_int32(const gchar *value, gint32 *out, GError **error);
> +gboolean type_cast_to_int64(const gchar *value, gint64 *out, GError **error);
> +gboolean type_cast_to_datetime_int(const gchar *value, guint64 *out, GError **error);
> +gboolean type_cast_to_datetime_str(const gchar *value, const char *format,
> + gchar **out, GError **error);
> +
> +#endif
> diff --git a/lib/value-pairs.c b/lib/value-pairs.c
> index e78fc58..dafd301 100644
> --- a/lib/value-pairs.c
> +++ b/lib/value-pairs.c
> @@ -26,6 +26,7 @@
> #include "vptransform.h"
> #include "logmsg.h"
> #include "templates.h"
> +#include "type-hinting.h"
> #include "cfg-parser.h"
> #include "misc.h"
> #include "scratch-buffers.h"
> @@ -151,16 +152,24 @@ value_pairs_add_glob_pattern(ValuePairs *vp, const gchar *pattern,
> vp->patterns[i] = p;
> }
>
> -void
> -value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, const gchar *value)
> +gboolean
> +value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg,
> + const gchar *key, TypeHint type,
> + const gchar *value, GError **error)
> {
> VPPairConf *p = g_new(VPPairConf, 1);
>
> p->name = g_strdup(key);
> p->template = log_template_new(cfg, NULL);
> - log_template_compile(p->template, value, NULL);
> + log_template_compile(p->template, value, error);
like I've written at the parser, this should probably get a LogTemplate
instance, perhaps a wrapper with this signature in case someone calls
value_pairs_add_pair() outside the parser context.
> + if (error && *error)
> + return FALSE;
> + p->template->type_hint = type;
this would get solved by the use of a LogTemplate * parameter.
> + if (error && *error)
> + return FALSE;
>
> g_ptr_array_add(vp->vpairs, p);
> + return TRUE;
> }
>
> static gchar *
> @@ -193,22 +202,20 @@ vp_pairs_foreach(gpointer data, gpointer user_data)
> LogMessage *msg = ((gpointer *)user_data)[2];
> gint32 seq_num = GPOINTER_TO_INT (((gpointer *)user_data)[3]);
> GTree *scope_set = ((gpointer *)user_data)[5];
> - SBGString *sb = sb_gstring_acquire();
> + SBTHGString *sb = sb_th_gstring_acquire();
> VPPairConf *vpc = (VPPairConf *)data;
>
> - log_template_format((LogTemplate *)vpc->template, msg, NULL, LTZ_LOCAL,
> - seq_num, NULL, sb_gstring_string(sb));
> + sb->type_hint = vpc->template->type_hint;
> + log_template_append_format((LogTemplate *)vpc->template, msg, NULL, LTZ_LOCAL,
> + seq_num, NULL, sb_th_gstring_string(sb));
>
> - if (!sb_gstring_string(sb)->str[0])
> + if (sb_th_gstring_string(sb)->len == 0)
> {
> - sb_gstring_release(sb);
> + sb_th_gstring_release(sb);
> return;
> }
>
> - g_tree_insert(scope_set, vp_transform_apply(vp, vpc->name),
> - sb_gstring_string(sb)->str);
> - g_string_steal(sb_gstring_string(sb));
> - sb_gstring_release(sb);
> + g_tree_insert(scope_set, vp_transform_apply(vp, vpc->name), sb);
if the actual formatting of the message would be delayed until our
callback actually needed it, we wouldn't need SBTHGString here. We'd
just store a LogTemplate reference in the tree.
> }
>
> /* runs over the LogMessage nv-pairs, and inserts them unless excluded */
> @@ -234,8 +241,11 @@ vp_msg_nvpairs_foreach(NVHandle handle, gchar *name,
> (log_msg_is_handle_sdata(handle) && (vp->scopes & VPS_SDATA))) ||
> inc)
> {
> - /* NOTE: the key is a borrowed reference in the hash, and value is freed */
> - g_tree_insert(scope_set, vp_transform_apply(vp, name), g_strndup(value, value_len));
> + SBTHGString *sb = sb_th_gstring_acquire();
> +
> + g_string_append_len(sb_th_gstring_string(sb), value, value_len);
> + sb->type_hint = TYPE_HINT_STRING;
> + g_tree_insert(scope_set, vp_transform_apply(vp, name), sb);
> }
I can see why this makes it difficult to use a LogTemplate instance in
the tree, however a LogTemplate could be created even for the case where
only a single nvpair is to be resolved, perhaps an optimization within
LogTemplate would eliminate the cost. (special casing for the
'single-value-reference' in the template string. This would probably
make things faster elsewhere too.
>
> return FALSE;
> @@ -246,7 +256,7 @@ static void
> vp_merge_set(ValuePairs *vp, LogMessage *msg, gint32 seq_num, ValuePairSpec *set, GTree *dest)
> {
> gint i;
> - SBGString *sb = sb_gstring_acquire();
> + SBTHGString *sb;
>
> for (i = 0; set[i].name; i++)
> {
> @@ -262,10 +272,12 @@ vp_merge_set(ValuePairs *vp, LogMessage *msg, gint32 seq_num, ValuePairSpec *set
> if (exclude)
> continue;
>
> + sb = sb_th_gstring_acquire();
> +
> switch (set[i].type)
> {
> case VPT_MACRO:
> - log_macro_expand(sb_gstring_string(sb), set[i].id, FALSE,
> + log_macro_expand(sb_th_gstring_string(sb), set[i].id, FALSE,
> NULL, LTZ_LOCAL, seq_num, NULL, msg);
> break;
> case VPT_NVPAIR:
> @@ -274,34 +286,55 @@ vp_merge_set(ValuePairs *vp, LogMessage *msg, gint32 seq_num, ValuePairSpec *set
> gssize len;
>
> nv = log_msg_get_value(msg, (NVHandle) set[i].id, &len);
> - g_string_append_len(sb_gstring_string(sb), nv, len);
> + g_string_append_len(sb_th_gstring_string(sb), nv, len);
> break;
> }
> default:
> g_assert_not_reached();
> }
>
> - if (!sb_gstring_string(sb)->str[0])
> - continue;
> + if (sb_th_gstring_string(sb)->len == 0)
> + {
> + sb_th_gstring_release(sb);
> + continue;
> + }
>
> - g_tree_insert(dest, vp_transform_apply(vp, set[i].name),
> - sb_gstring_string(sb)->str);
> - g_string_steal(sb_gstring_string(sb));
> + g_tree_insert(dest, vp_transform_apply(vp, set[i].name), sb);
> }
> - sb_gstring_release(sb);
> }
>
> -void
> +static gboolean
> +vp_foreach_helper (const gchar *name, const SBTHGString *hinted_value,
> + gpointer data)
> +{
> + VPForeachFunc func = ((gpointer *)data)[0];
> + gpointer user_data = ((gpointer *)data)[1];
> + gboolean *r = ((gpointer *)data)[2];
> +
> + *r &= !func(name, hinted_value->type_hint,
> + sb_th_gstring_string(hinted_value)->str, user_data);
I would enclose the type-hint and string values in an opaque VPValue
structure that our callback can pass back if the value is actually
needed and to query its type.
The number of arguments would be decreased for the callback and the
number of allocation could probably be decreased too.
> + return !*r;
> +}
> +
> +static void
> +vp_data_free (SBTHGString *s)
> +{
> + sb_th_gstring_release (s);
> +}
> +
> +gboolean
> value_pairs_foreach_sorted (ValuePairs *vp, VPForeachFunc func,
> GCompareDataFunc compare_func,
> LogMessage *msg, gint32 seq_num, gpointer user_data)
> {
> gpointer args[] = { vp, func, msg, GINT_TO_POINTER (seq_num), user_data, NULL };
> + gboolean result = TRUE;
> + gpointer helper_args[] = { func, user_data, &result };
> GTree *scope_set;
>
> scope_set = g_tree_new_full((GCompareDataFunc)compare_func, NULL,
> (GDestroyNotify)g_free,
> - (GDestroyNotify)g_free);
> + (GDestroyNotify)vp_data_free);
> args[5] = scope_set;
>
> /*
> @@ -328,18 +361,20 @@ value_pairs_foreach_sorted (ValuePairs *vp, VPForeachFunc func,
> g_ptr_array_foreach(vp->vpairs, (GFunc)vp_pairs_foreach, args);
>
> /* Aaand we run it through the callback! */
> - g_tree_foreach(scope_set, (GTraverseFunc)func, user_data);
> + g_tree_foreach(scope_set, (GTraverseFunc)vp_foreach_helper, helper_args);
>
> g_tree_destroy(scope_set);
> +
> + return result;
> }
>
> -void
> +gboolean
> value_pairs_foreach(ValuePairs *vp, VPForeachFunc func,
> LogMessage *msg, gint32 seq_num,
> gpointer user_data)
> {
> - value_pairs_foreach_sorted(vp, func, (GCompareDataFunc) strcmp,
> - msg, seq_num, user_data);
> + return value_pairs_foreach_sorted(vp, func, (GCompareDataFunc) strcmp,
> + msg, seq_num, user_data);
> }
>
> typedef struct
> @@ -483,7 +518,7 @@ vp_walker_name_split(vp_walk_stack_t **stack, vp_walk_state_t *state,
> }
>
> static gboolean
> -value_pairs_walker(const gchar *name, const gchar *value,
> +value_pairs_walker(const gchar *name, TypeHint type, const gchar *value,
> gpointer user_data)
> {
> vp_walk_state_t *state = (vp_walk_state_t *)user_data;
> @@ -496,10 +531,14 @@ value_pairs_walker(const gchar *name, const gchar *value,
> key = vp_walker_name_split (&st, state, name);
>
> if (st)
> - result = state->process_value(key, st->prefix, value, &st->data,
> + result = state->process_value(key, st->prefix,
> + type, value,
> + &st->data,
> state->user_data);
> else
> - result = state->process_value(key, NULL, value, NULL,
> + result = state->process_value(key, NULL,
> + type, value,
> + NULL,
> state->user_data);
>
> g_free(key);
> @@ -516,7 +555,7 @@ vp_walk_cmp(const gchar *s1, const gchar *s2)
> return strcmp(s2, s1);
> }
>
> -void
> +gboolean
> value_pairs_walk(ValuePairs *vp,
> VPWalkCallbackFunc obj_start_func,
> VPWalkValueCallbackFunc process_value_func,
> @@ -525,6 +564,7 @@ value_pairs_walk(ValuePairs *vp,
> gpointer user_data)
> {
> vp_walk_state_t state;
> + gboolean result;
>
> state.user_data = user_data;
> state.obj_start = obj_start_func;
> @@ -533,10 +573,12 @@ value_pairs_walk(ValuePairs *vp,
> state.stack = NULL;
>
> state.obj_start(NULL, NULL, NULL, NULL, NULL, user_data);
> - value_pairs_foreach_sorted(vp, value_pairs_walker,
> - (GCompareDataFunc)vp_walk_cmp, msg, seq_num, &state);
> + result = value_pairs_foreach_sorted(vp, value_pairs_walker,
> + (GCompareDataFunc)vp_walk_cmp, msg, seq_num, &state);
> vp_walker_stack_unwind_all(&state.stack, &state);
> state.obj_end(NULL, NULL, NULL, NULL, NULL, user_data);
> +
> + return result;
> }
>
> static void
> @@ -722,6 +764,32 @@ vp_cmdline_parse_rekey(const gchar *option_name, const gchar *value,
> return TRUE;
> }
>
> +static void
> +value_pairs_parse_type(gchar *spec, gchar **value, gchar **type)
> +{
> + char *sp, *ep;
> +
> + *type = NULL;
> +
> + sp = strchr(spec, '(');
> + if (sp == NULL)
> + {
> + *value = spec;
> + return;
> + }
> + ep = strchr(sp, ')');
> + if (ep == NULL || ep[1] != '\0')
> + {
> + *value = spec;
> + return;
> + }
> +
> + *value = sp + 1;
> + *type = spec;
> + sp[0] = '\0';
> + ep[0] = '\0';
> +}
> +
> static gboolean
> vp_cmdline_parse_pair (const gchar *option_name, const gchar *value,
> gpointer data, GError **error)
> @@ -729,7 +797,8 @@ vp_cmdline_parse_pair (const gchar *option_name, const gchar *value,
> gpointer *args = (gpointer *) data;
> ValuePairs *vp = (ValuePairs *) args[1];
> GlobalConfig *cfg = (GlobalConfig *) args[0];
> - gchar **kv;
> + gchar **kv, *v, *t;
> + TypeHint type;
>
> vp_cmdline_parse_rekey_finish (data);
> if (!g_strstr_len (value, strlen (value), "="))
> @@ -740,7 +809,17 @@ vp_cmdline_parse_pair (const gchar *option_name, const gchar *value,
> }
>
> kv = g_strsplit(value, "=", 2);
> - value_pairs_add_pair (vp, cfg, kv[0], kv[1]);
> + value_pairs_parse_type(kv[1], &v, &t);
> +
> + if (type_hint_parse(t, &type, error) != TRUE)
> + {
> + g_free(kv[0]);
> + g_free(kv[1]);
> + g_free(kv);
> + return FALSE;
> + }
> +
> + value_pairs_add_pair(vp, cfg, kv[0], type, v, NULL);
>
> g_free (kv[0]);
> g_free (kv[1]);
> diff --git a/lib/value-pairs.h b/lib/value-pairs.h
> index a6c9f34..75888c9 100644
> --- a/lib/value-pairs.h
> +++ b/lib/value-pairs.h
> @@ -27,12 +27,14 @@
>
> #include "syslog-ng.h"
> #include "nvtable.h"
> +#include "type-hinting.h"
>
> typedef struct _ValuePairs ValuePairs;
> -typedef gboolean (*VPForeachFunc)(const gchar *name, const gchar *value, gpointer user_data);
> +typedef gboolean (*VPForeachFunc)(const gchar *name, TypeHint type,
> + const gchar *value, gpointer user_data);
>
> typedef gboolean (*VPWalkValueCallbackFunc)(const gchar *name, const gchar *prefix,
> - const gchar *value,
> + TypeHint type, const gchar *value,
> gpointer *prefix_data, gpointer user_data);
> typedef gboolean (*VPWalkCallbackFunc)(const gchar *name,
> const gchar *prefix, gpointer *prefix_data,
> @@ -41,24 +43,26 @@ typedef gboolean (*VPWalkCallbackFunc)(const gchar *name,
>
> gboolean value_pairs_add_scope(ValuePairs *vp, const gchar *scope);
> void value_pairs_add_glob_pattern(ValuePairs *vp, const gchar *pattern, gboolean include);
> -void value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, const gchar *value);
> +gboolean value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg,
> + const gchar *key, TypeHint type,
> + const gchar *value, GError **error);
>
> void value_pairs_add_transforms(ValuePairs *vp, gpointer vpts);
>
> -void value_pairs_foreach_sorted(ValuePairs *vp, VPForeachFunc func,
> - GCompareDataFunc compare_func,
> - LogMessage *msg, gint32 seq_num,
> - gpointer user_data);
> -void value_pairs_foreach(ValuePairs *vp, VPForeachFunc func,
> - LogMessage *msg, gint32 seq_num,
> - gpointer user_data);
> +gboolean value_pairs_foreach_sorted(ValuePairs *vp, VPForeachFunc func,
> + GCompareDataFunc compare_func,
> + LogMessage *msg, gint32 seq_num,
> + gpointer user_data);
> +gboolean value_pairs_foreach(ValuePairs *vp, VPForeachFunc func,
> + LogMessage *msg, gint32 seq_num,
> + gpointer user_data);
>
> -void value_pairs_walk(ValuePairs *vp,
> - VPWalkCallbackFunc obj_start_func,
> - VPWalkValueCallbackFunc process_value_func,
> - VPWalkCallbackFunc obj_end_func,
> - LogMessage *msg, gint32 seq_num,
> - gpointer user_data);
> +gboolean value_pairs_walk(ValuePairs *vp,
> + VPWalkCallbackFunc obj_start_func,
> + VPWalkValueCallbackFunc process_value_func,
> + VPWalkCallbackFunc obj_end_func,
> + LogMessage *msg, gint32 seq_num,
> + gpointer user_data);
>
> ValuePairs *value_pairs_new(void);
> void value_pairs_free(ValuePairs *vp);
> diff --git a/modules/afamqp/afamqp.c b/modules/afamqp/afamqp.c
> index 923f90b..894f81d 100644
> --- a/modules/afamqp/afamqp.c
> +++ b/modules/afamqp/afamqp.c
> @@ -374,7 +374,8 @@ afamqp_dd_connect(AMQPDestDriver *self, gboolean reconnect)
> */
>
> static gboolean
> -afamqp_vp_foreach(const gchar *name, const gchar *value,
> +afamqp_vp_foreach(const gchar *name,
> + TypeHint type, const gchar *value,
> gpointer user_data)
> {
> amqp_table_entry_t **entries = (amqp_table_entry_t **) ((gpointer *)user_data)[0];
this would look like this:
afamqp_vp_foreach(const gchar *name, const VPValue *value, gpointer user_data)
{
type = vp_value_get_type(value);
value_string = vp_value_get_string(value);
}
vp_value_get_string() would in turn perform the actual formatting
of the template and not until this point.
Also VPValue could be allocated on the stack and LogTemplate would
contain the type and the ability to format it as a string.
> diff --git a/modules/afmongodb/afmongodb.c b/modules/afmongodb/afmongodb.c
> index 9779097..5fac90a 100644
> --- a/modules/afmongodb/afmongodb.c
> +++ b/modules/afmongodb/afmongodb.c
> @@ -359,7 +359,7 @@ afmongodb_vp_obj_end(const gchar *name,
>
> static gboolean
> afmongodb_vp_process_value(const gchar *name, const gchar *prefix,
> - const gchar *value,
> + TypeHint type, const gchar *value,
> gpointer *prefix_data, gpointer user_data)
> {
> bson *o;
> diff --git a/modules/json/format-json.c b/modules/json/format-json.c
> index d654c2e..edb1608 100644
> --- a/modules/json/format-json.c
> +++ b/modules/json/format-json.c
> @@ -163,7 +163,8 @@ tf_json_obj_end(const gchar *name,
> }
>
> static gboolean
> -tf_json_value(const gchar *name, const gchar *prefix, const gchar *value,
> +tf_json_value(const gchar *name, const gchar *prefix,
> + TypeHint type, const gchar *value,
> gpointer *prefix_data, gpointer user_data)
> {
> json_state_t *state = (json_state_t *)user_data;
> diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
> index ccd5e29..cf59ee1 100644
> --- a/tests/unit/Makefile.am
> +++ b/tests/unit/Makefile.am
> @@ -26,6 +26,7 @@ check_PROGRAMS = \
> test_serialize \
> test_zone \
> test_persist_state \
> + test_type_hints \
> test_value_pairs
>
> test_msgparse_SOURCES = test_msgparse.c
> diff --git a/tests/unit/test_type_hints.c b/tests/unit/test_type_hints.c
> new file mode 100644
> index 0000000..d453792
> --- /dev/null
> +++ b/tests/unit/test_type_hints.c
> @@ -0,0 +1,191 @@
> +#include "testutils.h"
> +#include "type-hinting.h"
> +#include "apphook.h"
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#define assert_type_hint(hint,expected) \
> + do \
> + { \
> + TypeHint t; \
> + GError *e = NULL; \
> + \
> + assert_true(type_hint_parse(hint, &t, &e), \
> + "Parsing '%s' as type hint", hint); \
> + \
> + assert_gint(t, expected, \
> + "Parsing '%s' as type hint results in correct type", hint); \
> + } while(0) \
> +
> +static void
> +assert_error(GError *error, gint code, const gchar *expected_message)
> +{
> + assert_not_null(error, "GError expected to be non-NULL");
> +
> + assert_gint(error->code, code, "GError error code is as expected");
> + if (expected_message)
> + assert_string(error->message, expected_message, "GError error message is as expected");
> +}
> +
> +static void
> +test_type_hint_parse(void)
> +{
> + TypeHint t;
> + GError *e = NULL;
> +
> + testcase_begin("Testing type hint parsing");
> +
> + assert_type_hint(NULL, TYPE_HINT_STRING);
> + assert_type_hint("string", TYPE_HINT_STRING);
> + assert_type_hint("literal", TYPE_HINT_LITERAL);
> + assert_type_hint("boolean", TYPE_HINT_BOOLEAN);
> + assert_type_hint("int", TYPE_HINT_INT32);
> + assert_type_hint("int32", TYPE_HINT_INT32);
> + assert_type_hint("int64", TYPE_HINT_INT64);
> + assert_type_hint("datetime", TYPE_HINT_DATETIME);
> + assert_type_hint("default", TYPE_HINT_DEFAULT);
> +
> + assert_false(type_hint_parse("invalid-hint", &t, &e),
> + "Parsing an invalid hint results in an error.");
> +
> + assert_error(e, TYPE_HINTING_INVALID_TYPE, "invalid-hint");
> +
> + testcase_end();
> +}
> +
> +#define assert_type_cast(target,value,out) \
> + do \
> + { \
> + assert_true(type_cast_to_##target(value, out, &error), \
> + "Casting '%s' to %s works", value, #target); \
> + assert_no_error(error, "Successful casting returns no error"); \
> + } while(0)
> +
> +#define assert_type_cast_fail(target,value,out) \
> + do \
> + { \
> + assert_false(type_cast_to_##target(value, out, &error), \
> + "Casting '%s' to %s fails", value, #target); \
> + assert_error(error, TYPE_HINTING_INVALID_CAST, NULL); \
> + error = NULL; \
> + } while(0)
> +
> +#define assert_bool_cast(value,expected) \
> + do \
> + { \
> + gboolean ob; \
> + assert_type_cast(boolean, value, &ob); \
> + assert_gboolean(ob, expected, "'%s' casted to boolean is %s", \
> + value, #expected); \
> + } while(0)
> +
> +#define assert_int_cast(value,width,expected) \
> + do \
> + { \
> + gint##width i; \
> + assert_type_cast(int##width, value, &i); \
> + assert_gint##width(i, expected, "'%s' casted to int%s is %u", \
> + value, expected); \
> + } while(0) \
> +
> +static void
> +test_type_cast(void)
> +{
> + GError *error = NULL;
> + gboolean ob;
> + gint32 i32;
> + gint64 i64;
> + guint64 dt;
> +
> + testcase_begin("Testing type casting");
> +
> + /* Boolean */
> +
> + assert_bool_cast("True", TRUE);
> + assert_bool_cast("true", TRUE);
> + assert_bool_cast("1", TRUE);
> + assert_bool_cast("totally true", TRUE);
> + assert_bool_cast("False", FALSE);
> + assert_bool_cast("false", FALSE);
> + assert_bool_cast("0", FALSE);
> + assert_bool_cast("fatally false", FALSE);
> +
> + assert_type_cast_fail(boolean, "booyah", &ob);
> +
> + /* int32 */
> + assert_int_cast("12345", 32, 12345);
> + assert_type_cast_fail(int32, "12345a", &i32);
> +
> + /* int64 */
> + assert_int_cast("12345", 64, 12345);
> + assert_type_cast_fail(int64, "12345a", &i64);
> +
> + /* datetime */
> + assert_type_cast(datetime_int, "12345", &dt);
> + assert_guint64(dt, 12345000, "Casting '12345' to datetime works");
> + assert_type_cast(datetime_int, "12345.5", &dt);
> + assert_guint64(dt, 12345500, "Casting '12345.5' to datetime works");
> + assert_type_cast(datetime_int, "12345.54", &dt);
> + assert_guint64(dt, 12345540, "Casting '12345.54' to datetime works");
> + assert_type_cast(datetime_int, "12345.543", &dt);
> + assert_guint64(dt, 12345543, "Casting '12345.543' to datetime works");
> + assert_type_cast(datetime_int, "12345.54321", &dt);
> + assert_guint64(dt, 12345543, "Casting '12345.54321' to datetime works");
> +
> + assert_type_cast_fail(datetime_int, "invalid", &dt);
> +
> + testcase_end();
> +}
> +
> +#define assert_type_cast_parse_strictness(strictness,expected) \
> + do \
> + { \
> + gint r; \
> + \
> + assert_true(type_cast_strictness_parse(strictness, &r, &error), \
> + "Parsing '%s' works", strictness); \
> + assert_no_error(error, "Successful strictness parsing returns no error"); \
> + assert_gint32(r, expected, "'%s' parses down to '%s'", strictness, #expected); \
> + } while(0)
> +
> +static void
> +test_type_cast_strictness(void)
> +{
> + GError *error = NULL;
> + gint r;
> +
> + testcase_begin("Testing type cast strictness");
> +
> + assert_type_cast_parse_strictness("drop-message", TYPE_CAST_DROP_MESSAGE);
> + assert_type_cast_parse_strictness("silently-drop-message",
> + TYPE_CAST_DROP_MESSAGE | TYPE_CAST_SILENTLY);
> +
> + assert_type_cast_parse_strictness("drop-property", TYPE_CAST_DROP_PROPERTY);
> + assert_type_cast_parse_strictness("silently-drop-property",
> + TYPE_CAST_DROP_PROPERTY | TYPE_CAST_SILENTLY);
> +
> + assert_type_cast_parse_strictness("fallback-to-string", TYPE_CAST_FALLBACK_TO_STRING);
> + assert_type_cast_parse_strictness("silently-fallback-to-string",
> + TYPE_CAST_FALLBACK_TO_STRING | TYPE_CAST_SILENTLY);
> +
> + assert_false(type_cast_strictness_parse("do-what-i-mean", &r, &error),
> + "The 'do-what-i-mean' strictness is not recognised");
> + assert_error(error, TYPE_CAST_INVALID_STRICTNESS, NULL);
> +
> + testcase_end();
> +}
> +
> +int
> +main (void)
> +{
> + app_startup();
> +
> + test_type_hint_parse();
> + test_type_cast();
> + test_type_cast_strictness();
> +
> + app_shutdown();
> +
> + return 0;
> +}
> diff --git a/tests/unit/test_value_pairs.c b/tests/unit/test_value_pairs.c
> index ca4ba32..f857ca8 100644
> --- a/tests/unit/test_value_pairs.c
> +++ b/tests/unit/test_value_pairs.c
> @@ -10,7 +10,7 @@
> gboolean success = TRUE;
>
> gboolean
> -vp_keys_foreach(const gchar *name, const gchar *value, gpointer user_data)
> +vp_keys_foreach(const gchar *name, TypeHint type, const gchar *value, gpointer user_data)
> {
> gpointer *args = (gpointer *) user_data;
> GList **keys = (GList **) args[0];
> @@ -62,7 +62,7 @@ testcase(const gchar *scope, const gchar *exclude, const gchar *expected, GPtrAr
> value_pairs_add_scope(vp, scope);
> if (exclude)
> value_pairs_add_glob_pattern(vp, exclude, FALSE);
> - value_pairs_add_pair(vp, configuration, "test.key", "$MESSAGE");
> + value_pairs_add_pair(vp, configuration, "test.key", TYPE_HINT_STRING, "$MESSAGE", NULL);
>
> if (transformers)
> {
> @@ -123,7 +123,7 @@ main(int argc, char *argv[])
>
> testcase("all-nv-pairs", NULL, ".SDATA.EventData at 18372.4.Data,.SDATA.Keywords at 18372.4.Keyword,.SDATA.meta.sequenceId,.SDATA.meta.sysUpTime,.SDATA.origin.ip,HOST,MESSAGE,MSGID,PID,PROGRAM", NULL);
>
> - testcase("everything", NULL, ".SDATA.EventData at 18372.4.Data,.SDATA.Keywords at 18372.4.Keyword,.SDATA.meta.sequenceId,.SDATA.meta.sysUpTime,.SDATA.origin.ip,AMPM,BSDTAG,C_DATE,C_DAY,C_FULLDATE,C_HOUR,C_ISODATE,C_MIN,C_MONTH,C_MONTH_ABBREV,C_MONTH_NAME,C_MONTH_WEEK,C_SEC,C_STAMP,C_TZ,C_TZOFFSET,C_UNIXTIME,C_WEEK,C_WEEKDAY,C_WEEK_DAY,C_WEEK_DAY_ABBREV,C_WEEK_DAY_NAME,C_YEAR,C_YEAR_DAY,DATE,DAY,FACILITY,FACILITY_NUM,FULLDATE,HOST,HOUR,HOUR12,ISODATE,LEVEL,LEVEL_NUM,LOGHOST,MESSAGE,MIN,MONTH,MONTH_ABBREV,MONTH_NAME,MONTH_WEEK,MSEC,MSG,MSGHDR,MSGID,PID,PRI,PRIORITY,PROGRAM,R_AMPM,R_DATE,R_DAY,R_FULLDATE,R_HOUR,R_HOUR12,R_ISODATE,R_MIN,R_MONTH,R_MONTH_ABBREV,R_MONTH_NAME,R_MONTH_WEEK,R_MSEC,R_SEC,R_STAMP,R_TZ,R_TZOFFSET,R_UNIXTIME,R_USEC,R_WEEK,R_WEEKDAY,R_WEEK_DAY,R_WEEK_DAY_ABBREV,R_WEEK_DAY_NAME,R_YEAR,R_YEAR_DAY,SDATA,SEC,SEQNUM,SOURCEIP,STAMP,SYSUPTIME,S_AMPM,S_DATE,S_DAY,S_FULLDATE,S_HOUR,S_HOUR12,S_ISODATE,S_MIN,S_MONTH,S_MONTH_ABBREV,S_MONTH_NAME,S_MONTH_WEEK,S_MSEC,S_SEC
,S
> _STAMP,S
> _TZ,S_TZOFFSET,S_UNIXTIME,S_USEC,S_WEEK,S_WEEKDAY,S_WEEK_DAY,S_WEEK_DAY_ABBREV,S_WEEK_DAY_NAME,S_YEAR,S_YEAR_DAY,TAG,TAGS,TZ,TZOFFSET,UNIXTIME,USEC,WEEK,WEEKDAY,WEEK_DAY,WEEK_DAY_ABBREV,WEEK_DAY_NAME,YEAR,YEAR_DAY", NULL);
> + testcase("everything", NULL, ".SDATA.EventData at 18372.4.Data,.SDATA.Keywords at 18372.4.Keyword,.SDATA.meta.sequenceId,.SDATA.meta.sysUpTime,.SDATA.origin.ip,AMPM,BSDTAG,CONTEXT_ID,C_DATE,C_DAY,C_FULLDATE,C_HOUR,C_ISODATE,C_MIN,C_MONTH,C_MONTH_ABBREV,C_MONTH_NAME,C_MONTH_WEEK,C_SEC,C_STAMP,C_TZ,C_TZOFFSET,C_UNIXTIME,C_WEEK,C_WEEKDAY,C_WEEK_DAY,C_WEEK_DAY_ABBREV,C_WEEK_DAY_NAME,C_YEAR,C_YEAR_DAY,DATE,DAY,FACILITY,FACILITY_NUM,FULLDATE,HOST,HOUR,HOUR12,ISODATE,LEVEL,LEVEL_NUM,LOGHOST,MESSAGE,MIN,MONTH,MONTH_ABBREV,MONTH_NAME,MONTH_WEEK,MSEC,MSG,MSGHDR,MSGID,PID,PRI,PRIORITY,PROGRAM,R_AMPM,R_DATE,R_DAY,R_FULLDATE,R_HOUR,R_HOUR12,R_ISODATE,R_MIN,R_MONTH,R_MONTH_ABBREV,R_MONTH_NAME,R_MONTH_WEEK,R_MSEC,R_SEC,R_STAMP,R_TZ,R_TZOFFSET,R_UNIXTIME,R_USEC,R_WEEK,R_WEEKDAY,R_WEEK_DAY,R_WEEK_DAY_ABBREV,R_WEEK_DAY_NAME,R_YEAR,R_YEAR_DAY,SDATA,SEC,SEQNUM,SOURCEIP,STAMP,SYSUPTIME,S_AMPM,S_DATE,S_DAY,S_FULLDATE,S_HOUR,S_HOUR12,S_ISODATE,S_MIN,S_MONTH,S_MONTH_ABBREV,S_MONTH_NAME,S_MONTH_WEEK,S
_M
> SEC,S_SE
> C,S_STAMP,S_TZ,S_TZOFFSET,S_UNIXTIME,S_USEC,S_WEEK,S_WEEKDAY,S_WEEK_DAY,S_WEEK_DAY_ABBREV,S_WEEK_DAY_NAME,S_YEAR,S_YEAR_DAY,TAG,TAGS,TZ,TZOFFSET,UNIXTIME,USEC,WEEK,WEEKDAY,WEEK_DAY,WEEK_DAY_ABBREV,WEEK_DAY_NAME,YEAR,YEAR_DAY", NULL);
>
> testcase("nv-pairs", ".SDATA.*", "HOST,MESSAGE,MSGID,PID,PROGRAM", NULL);
>
> @@ -139,7 +139,7 @@ main(int argc, char *argv[])
> g_ptr_array_add(transformers, value_pairs_new_transform_shift(2));
> g_ptr_array_add(transformers, value_pairs_new_transform_replace("C_", "CC_"));
>
> - testcase("everything", NULL, ".SDATA.EventData at 18372.4.Data,.SDATA.Keywords at 18372.4.Keyword,.SDATA.meta.sequenceId,.SDATA.meta.sysUpTime,.SDATA.origin.ip,AMPM,BSDTAG,CC_DATE,CC_DAY,CC_FULLDATE,CC_HOUR,CC_ISODATE,CC_MIN,CC_MONTH,CC_MONTH_ABBREV,CC_MONTH_NAME,CC_MONTH_WEEK,CC_SEC,CC_STAMP,CC_TZ,CC_TZOFFSET,CC_UNIXTIME,CC_WEEK,CC_WEEKDAY,CC_WEEK_DAY,CC_WEEK_DAY_ABBREV,CC_WEEK_DAY_NAME,CC_YEAR,CC_YEAR_DAY,DATE,DAY,FACILITY,FACILITY_NUM,FULLDATE,HOST,HOUR,HOUR12,ISODATE,LEVEL,LEVEL_NUM,LOGHOST,MESSAGE,MIN,MONTH,MONTH_ABBREV,MONTH_NAME,MONTH_WEEK,MSEC,MSG,MSGHDR,MSGID,PID,PRI,PRIORITY,PROGRAM,R_AMPM,R_DATE,R_DAY,R_FULLDATE,R_HOUR,R_HOUR12,R_ISODATE,R_MIN,R_MONTH,R_MONTH_ABBREV,R_MONTH_NAME,R_MONTH_WEEK,R_MSEC,R_SEC,R_STAMP,R_TZ,R_TZOFFSET,R_UNIXTIME,R_USEC,R_WEEK,R_WEEKDAY,R_WEEK_DAY,R_WEEK_DAY_ABBREV,R_WEEK_DAY_NAME,R_YEAR,R_YEAR_DAY,SDATA,SEC,SEQNUM,SOURCEIP,STAMP,SYSUPTIME,S_AMPM,S_DATE,S_DAY,S_FULLDATE,S_HOUR,S_HOUR12,S_ISODATE,S_MIN,S_MONTH,S_MONTH_ABBREV,S_MONTH_NAME,S_M
ON
> TH_WEEK,
> S_MSEC,S_SEC,S_STAMP,S_TZ,S_TZOFFSET,S_UNIXTIME,S_USEC,S_WEEK,S_WEEKDAY,S_WEEK_DAY,S_WEEK_DAY_ABBREV,S_WEEK_DAY_NAME,S_YEAR,S_YEAR_DAY,TAG,TAGS,TZ,TZOFFSET,UNIXTIME,USEC,WEEK,WEEKDAY,WEEK_DAY,WEEK_DAY_ABBREV,WEEK_DAY_NAME,YEAR,YEAR_DAY", transformers);
> + testcase("everything", NULL, ".SDATA.EventData at 18372.4.Data,.SDATA.Keywords at 18372.4.Keyword,.SDATA.meta.sequenceId,.SDATA.meta.sysUpTime,.SDATA.origin.ip,AMPM,BSDTAG,CC_DATE,CC_DAY,CC_FULLDATE,CC_HOUR,CC_ISODATE,CC_MIN,CC_MONTH,CC_MONTH_ABBREV,CC_MONTH_NAME,CC_MONTH_WEEK,CC_SEC,CC_STAMP,CC_TZ,CC_TZOFFSET,CC_UNIXTIME,CC_WEEK,CC_WEEKDAY,CC_WEEK_DAY,CC_WEEK_DAY_ABBREV,CC_WEEK_DAY_NAME,CC_YEAR,CC_YEAR_DAY,CONTEXT_ID,DATE,DAY,FACILITY,FACILITY_NUM,FULLDATE,HOST,HOUR,HOUR12,ISODATE,LEVEL,LEVEL_NUM,LOGHOST,MESSAGE,MIN,MONTH,MONTH_ABBREV,MONTH_NAME,MONTH_WEEK,MSEC,MSG,MSGHDR,MSGID,PID,PRI,PRIORITY,PROGRAM,R_AMPM,R_DATE,R_DAY,R_FULLDATE,R_HOUR,R_HOUR12,R_ISODATE,R_MIN,R_MONTH,R_MONTH_ABBREV,R_MONTH_NAME,R_MONTH_WEEK,R_MSEC,R_SEC,R_STAMP,R_TZ,R_TZOFFSET,R_UNIXTIME,R_USEC,R_WEEK,R_WEEKDAY,R_WEEK_DAY,R_WEEK_DAY_ABBREV,R_WEEK_DAY_NAME,R_YEAR,R_YEAR_DAY,SDATA,SEC,SEQNUM,SOURCEIP,STAMP,SYSUPTIME,S_AMPM,S_DATE,S_DAY,S_FULLDATE,S_HOUR,S_HOUR12,S_ISODATE,S_MIN,S_MONTH,S_MONTH_ABBREV,S_MON
TH
> _NAME,S_
> MONTH_WEEK,S_MSEC,S_SEC,S_STAMP,S_TZ,S_TZOFFSET,S_UNIXTIME,S_USEC,S_WEEK,S_WEEKDAY,S_WEEK_DAY,S_WEEK_DAY_ABBREV,S_WEEK_DAY_NAME,S_YEAR,S_YEAR_DAY,TAG,TAGS,TZ,TZOFFSET,UNIXTIME,USEC,WEEK,WEEKDAY,WEEK_DAY,WEEK_DAY_ABBREV,WEEK_DAY_NAME,YEAR,YEAR_DAY", transformers);
> g_ptr_array_free(transformers, TRUE);
>
> app_shutdown();
This patch would need some work. I have already integrated the
scratch-buffers stuff, but I wouldn't this one.
--
Bazsi
More information about the syslog-ng
mailing list