[collectd] Generalized ignorelist functionality

[Luboš Staněk]

Hi Florian,

Florian Forster napsal(a):
> Hi Lubos,
> 
> I'm in the process of looking over your code right this moment ;) I've
> renamed `config_list.[ch]' to `utils_ignorelist.[ch]' because I think
> that's more intuitive. This inverts the functionality to a point, so
> some more small changes would make sense, to get the overall interface
> more streamlined..
> 

I agree.


>>> I'll see over it more thoroughly tomorrow and maybe I have some
>>> points then; for now I'm hapy that is compiles cleanly :)
>> Sure, you will. :)
> 
> And here it comes ;)

Of course. :)


> - I've inverted the argument passed to `create' and renamed it to
>   `invert'. If it is false, all matching entries will be ignored. If if
>   is true, all matching entries will be collected, everything else will
>   be ignored.
>   Come to think about it, this is the other way around as the usual
>   config-usage (where you have to set `IgnoreSelected' to get an
>   `ignorelist' functionality. What do you think, should I undo my
>   changes and rename it to `search list' (or some other, ``forward
>   matching'' name) instead?

I like the `invert'.


> - Is the `init' function necessary? If there was polymorphism in C I'd
>   welcome having a default-argument for the `create' function, but think
>   think having both, `init' and `create' with basically the same
>   functionality, is confusing.

Because you proposed the "ignorelist_t *ignorelist_create (int
inverse);". The state of the "ignore" is commonly not known to the end
of the options. Therefore I have created the function not requiring a
parameter. It can be omitted.


> - Why did you make the regex-stuff optional? The regex-things are in
>   POSIX and should therefore exist everywhere. (How well that works can
>   be seen in `utils_mount.c' *sigh*) Anyway, if someone only writes
>   `Match foobar' the regex-automaton will essentially do the same as
>   `strcmp'..

I do not know much about portability of the POSIX stuff to some marginal
systems like old HP-UX or Solaris. Therefore the regex is selectable to
be able to compile collectd without it. If you think it is really
portable, you can remove the selection.


>   Also, we would not need the delimiters then, which I don't like that
>   much. Alternatively we could provide some `add_regex' function for
>   plugins to provide both, `MatchString' and `MatchRegex'.
>   The advantage of string-comparsion is, that it's absolutely ordered,
>   making is possible to use a binary search tree and increase speed. But
>   with only a few entries this speedup is likely neglectible.
> 

The real speed of the regex is with the precompiled regexes. We need to
know that it is the regex and not only string to `strcmp'.
I wrote in the previous message:
>>   I wanted to differentiate the string and the regex without adding new
>>   config commands. Therefore I used '|' as a delimiter. It is possible to
>>   use any ('/' as you offered). But maybe adding a config command like
>>   ...RE (SensorsRE) would be better for reading the collectd.conf.

Do you have another idea how to differentiate strings and regexes?

It is not a problem to export another function for an `add_regex', the
code is internally divided into two `add' functions. But it would end in
the redundant code in plugins to differentiate string and regex.

I think that the best compromise is the config's regex command like
SensorsRE or InterfaceRE or something similar.

Best regards,
Lubos