Fix variables case sensitiveness #1810
Conversation
|
Tested with |
| @@ -106,7 +106,7 @@ void InMemoryPerProcess::resolveMultiMatches(const std::string& var, | |||
| } | |||
| } else { | |||
| for (auto &a : *this) { | |||
| if (a.first.compare(0, var.size(), var) == 0) { | |||
| if (strncasecmp(a.first.c_str(), var.c_str(), a.first.size()) == 0) { | |||
There was a problem hiding this comment.
Ignoring the change from case-sensitive to case-insensitive (which is the correct fix):
The old code compares the first var.size() characters, the new code compares the first a.first.size() characters. If these two sizes are not the same, this is not equivalent.
Consider var = "tx.paranoia", key = "tx.paranoia_level": the old comparison is true, the new one false.
I don't know why the logic was changed from full comparison to partial comparison in the first place (it looks wrong to me), but with this commit we're changing the logic again.
It would be good to learn about the motivation of the original change (892beb5). One aspect is that it turned a hash lookup into a linear search, so it may well have performance impact.
There was a problem hiding this comment.
Consider var = "tx.paranoia", key = "tx.paranoia_level": the old comparison is true, the new one false.
Sorry for the question, but why is the old way accepted? In my opinion, any strNcmp is incomprehensible. If I want to set a variable with name tx.paranoia, I want to search it exactly, not the ones which starts with it. :)
I mean, the old any new method is wrong :)
There was a problem hiding this comment.
@airween : I agree with you, as I said in my comment. For the purpose of this PR, however, I was just pointing out the change in logic that is not related to case sensitivity, which is the only thing this commit should change.
I've raised the question of comparing only the first N bytes on the original PR.
There was a problem hiding this comment.
@michaelgranzow-avi : sorry, may be you misunderstood me. I don't understand why is there any strn... comparing, instead of "simple" strcmp()/strcasecmp()? Why just need to compare the first N byte, why not the two strings as they are, without any length argument?
|
fixed by: d810de9 |
Changes the behaviour of the collection backend to fix an issue where variables were case sensitive in some scenarios. Should fix #1808.