[syslog-ng] [PATCH 1/3] value-pairs: Type hinting support

[Gergely Nagy]

Balazs Scheidler <bazsi77 at gmail.com> writes:

>> +/* START_RULES */
>> +
>> +template_content
>> +               : string                                                 { $$.str = $1; $$.typehint =
>> NULL; } +               | LL_IDENTIFIER '(' string ')'     { $$.str = $3;
>> $$.typehint = $1; } +               ;
>> +
>
> can't we move template compilation here somehow? e.g. return
> logtemplate as a ptr?

We probably can. I originally had it written in a very different way,
and forgot to simplify it further.

>> +log_template_set_type_hint(LogTemplate *self, const gchar *hint, GError
>> **error) +{
>> +   if (hint == NULL)
>> +       return TRUE;
>> +
>> +   if (strcmp(hint, "string") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_STRING;
>> +   else if (strcmp(hint, "int32") == 0 || strcmp(hint, "int") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_INT32;
>> +   else if (strcmp(hint, "int64") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_INT64;
>> +   else if (strcmp(hint, "datetime") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_DATETIME;
>> +   else if (strcmp(hint, "boolean") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_BOOLEAN;
>> +   else if (strcmp(hint, "default") == 0)
>> +       self->type_hint = TEMPLATE_TYPE_DEFAULT;
>> +   else
>> +       {
>> +           g_set_error(error, LOG_TEMPLATE_ERROR, LOG_TEMPLATE_ERROR_TYPE,
>> +                                   "%s", hint);
>> +           return FALSE;
>> +       }
>> +
>> +   return TRUE;
>> +}
>
> I'm thinking about whether it'd make sense to configure this at
> runtime, and whether the list of supported values could be destination
> specific.

I think we should have a centrally governed list, both because that's
simpler, and because there's only so many times our current drivers can
potentially support. I don't really see a need to make it extensible at
this point.

If the need arises, we can rework the code, until then, I don't really
see the point.

>> +typedef enum
>> +{
>> +   TEMPLATE_TYPE_STRING,
>> +   TEMPLATE_TYPE_BOOLEAN,
>> +   TEMPLATE_TYPE_INT32,
>> +   TEMPLATE_TYPE_INT64,
>> +   TEMPLATE_TYPE_DATETIME,
>> +   TEMPLATE_TYPE_DEFAULT,
>> +} LogTemplateType;
>> +
>
> default is somehow an exception here.

Mhm. That's not really a type, indeed. Perhaps it should be lifted out,
and implemented separately from the type hinting system?


-- 
|8]