|
Responding to Robin's comments:
> - there's code to remove quotes, but that seems to work only if there's just a single variable in the env_vars (i.e., not if individual variables are quoted). Is that intended? What's the situation here that's addressed, i.e, why would there be quotes in the first place?
The code to remove quotes is intended to handle this situation: env_vars="VAR1=1 VAR2=2" I thought someone might try this because they might think the quotes are needed (they're not needed) due to the space between the two environment variables. In this case, if we don't remove the quotes, then the shell interprets "VAR1=1 VAR2=2" as a command (which fails obviously, but the user wouldn't get useful feedback of what's happening, so that's why I'm just removing such quotes if they are found).
> - I suggest changing the node's env_vars to a dictionary internally. Maintaining the space-separated list seems error-prone, in particular when plugins modify it. _makeEnvParams() would then build the string. That could also solve the quotation problem.
That would make the debug output easier to read (no chance of having duplicate environment variables in the output in the case where a user tries to override a value set by a plugin), but I should mention that the space-separated list is user-supplied (in node.cfg, we'd have this: env_vars=VAR1=1 VAR2=2), so if we use a dictionary then we'd first need to separate that list. Another approach would be to force users to specify multiple env_vars entries (one for each environment variable), like this: env_vars=VAR1=1 env_vars=VAR2=2 But, currently the config file parser only uses the last entry and ignores all previous values (so in this example Bro would see VAR2=2, but no variable named VAR1), so the config file parser would need to be changed to support this.
> - the Myricom plugin is overriding the existing variables, that should probably be adding to them instead?
Not sure I understand. Are you saying that the myricom plugin is doing something different than the PF_RING plugin? I think both plugins are behaving the same way (i.e., if a user sets "XYZ=3" in node.cfg, and a plugin sets "XYZ=15", then Bro sees "XYZ=3").
> - I'm not seeing the broctl.cfg option?
Right, I did not implement that feature yet.
> - The option documentation should specify the format.
I've added a usage example in the documentation.
|