On 11/10/2012 04:18 PM, Gergely Nagy wrote:
balint.kovacs@balabit.com writes:
This patch implements an inherit-name-value-pairs attribute for message type actions is patterndb correlation. In case this is set to TRUE, the triggering log message is cloned and all name-value pairs are automagically copied to the emitted message. The values set in the values XML subtree overwrite the original values. In case it is set to FALSE or unspecified, a new message is generated and only the values set manually are added to it. This looks simple, and is backwards compatible, I like it. Picked it to my feature/3.4/dbparser/corellation-improvements branch for further testing. Kewl, thanks.
See my comments below, though.
diff --git a/modules/dbparser/patterndb.c b/modules/dbparser/patterndb.c index 8720ea0..168a24c 100644 --- a/modules/dbparser/patterndb.c +++ b/modules/dbparser/patterndb.c [...] +void +pdb_action_set_inheritance(PDBAction *self, const gchar *inherit_nvpairs, GError **error) +{ + if (strcmp(inherit_nvpairs, "TRUE") == 0) + self->inherit_nvpairs = TRUE; + else if (strcmp(inherit_nvpairs, "FALSE") == 0) + self->inherit_nvpairs = FALSE; + else + g_set_error(error, 0, 1, "Unknown inheritance type: %s", inherit_nvpairs); +} Although there is no similar thing in patterndb yet (boolean flags of this kind), the xs:boolean type of XML can be "true", "false", "0", or "1", so we should - in my opinion - accept all of those, case insensitively. A simple way would be something like:
if (inherit_nvpairs[0] == 't' || inherit_nvpairs[0] == 'T' || inherit_nvpairs[0] == '1') self->inerhit_nvpairs = TRUE; else if (inherit_nvpairs[0] == 'f' || inherit_nvpairs[0] == 'F' || inherit_nvpairs[0] == '0') self->inherit_nvpairs = FALSE; else g_set_error(error, 0, 1, "Unknown inheritance type: %s", inherit_nvpairs);
What do you think?
This'd allow things like inherit-name-value-pairs="FOR SURE" (and parse it as false), but I can live with that, as long as the valid cases are parsed correctly.
This should be added to the XSD aswell, so that patterndb files using it can still pass validation.
[...] Thanks for pointing this out, this is of course more compatible, intuitive, so in short - better.
+ if (action->inherit_nvpairs) + { + LogPathOptions path_options = LOG_PATH_OPTIONS_INIT; + path_options.ack_needed = FALSE; + genmsg = log_msg_clone_cow(msg, &path_options); + } + else + { + genmsg = log_msg_new_empty(); + genmsg->flags |= LF_LOCAL; + genmsg->timestamps[LM_TS_STAMP] = msg->timestamps[LM_TS_STAMP]; + } As you mentioned in person, valgrind was a bit upset about the log_msg_clone_cow() here, I'll investigate that once I've written some test cases.
Which leads us to the next point: tests! I wants them. But since I need to learn patterndb aswell, I'll write them.
Great, thanks again, I'm quite short on time these days. Let me know if I can help. Balint