On Jun 5, 2008, at 11:10 PM, Dale King wrote:
I saw that when you made the checkin, but I don't understand why you
think it is only a cosmetic bug. It seems to me that if you were
trying to configure a syslog appender for LOCAL2 for instance it would
call setFacility with LOCAL2 as the facility name. This would call
getFacility( "LOCAL2" ) which will not find a mapping for "LOCAL2" and
instead of returning the value LOG_LOCAL2 as it should it will return
LOG_UNDEF. This will cause the facility to default to USER.
I don't know anything about syslog to know what the ramifications of
that are, but I don't see how you can claim this behavior is merely
cosmetic.
The code fragment under question is:
else if (StringHelper::equalsIgnoreCase(s,
LOG4CXX_STR("LOCAL0"), LOG4CXX_STR("local0")))
{
return LOG_LOCAL0;
}
else if (StringHelper::equalsIgnoreCase(s,
LOG4CXX_STR("LOCAL1"), LOG4CXX_STR("local1")))
{
return LOG_LOCAL1;
}
else if (StringHelper::equalsIgnoreCase(s,
LOG4CXX_STR("LOCAL1"), LOG4CXX_STR("local2")))
{
return LOG_LOCAL2;
}
else if (StringHelper::equalsIgnoreCase(s,
LOG4CXX_STR("LOCAL1"), LOG4CXX_STR("local3")))
{
...
Where the second argument of equalsIgnoreCase is "LOCAL1" where it
wasn't intended to be.
equalsIgnoreCase is essentially:
if (arg1.size() != arg2.size() || arg1.size() != arg2) return false;
for(int i = 0; i < arg1.size(); i++) {
if (arg1[i] != arg2[i] && arg1[i] != arg3[i]) return false;
}
return true;
if arg1 is "LOCAL1", "local1", "LoCaL1" or any other variant, it will
return LOG_LOCAL1, since it will match the second else if. It will
not fall through to the 3 else if or later that would incorrectly
match since they have 1 as one of the 2 options for the 6 character.
If arg2 is "LOCAL2", "local2", "LoCal2" or any other variant, it will
return LOG_LOCAL2. equalsIgnoreCase only has to have a match on one
of the two potential characters and so as long as one of the matching
strings has the right number, it will catch all the expected matches.
All of the incorrect matches would already have been eliminated by the
second else if.
Basically, the misplaced "LOCAL1" adds the potential for false matches
in the later clauses, but any potential false match has already been
matched.
