[collectd] Generalized ignorelist functionality
lubek at users.sourceforge.net
Sat Nov 18 14:48:20 CET 2006
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'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
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.
More information about the collectd