You are viewing a plain text version of this content. The canonical link for it is here.
Posted to log4cxx-dev@logging.apache.org by Andreas Fester <af...@apache.org> on 2007/01/09 22:51:09 UTC
Unnecessary comparisons
Hi,
I am currently trying to fix the last remaining issues for LOGCXX-14,
to get a clean build even with pedantic compiler warnings enabled.
Besides a few const/non-const casting issues, the last two are
"comparison of unsigned expression < 0 is always false"
in bytebuffer.cpp and nameabbreviator.cpp (see patches below).
IMHO the compiler is correct here, since both size_t and
basic_string::size_type (typedef'd as LogString::size_type)
are unsigned types.
Also, the fix in bytebuffer should not be critical, because it
is "only" an argument guard. I also followed the control flow in
nameabbreviator.cpp and did not find a case where "pos" could ever
become negative.
Any comments or objections on these patches?
Thanks & Best Regards,
Andreas
Index: src/bytebuffer.cpp
===================================================================
--- src/bytebuffer.cpp (Revision 494555)
+++ src/bytebuffer.cpp (Arbeitskopie)
@@ -48,7 +48,7 @@
}
void ByteBuffer::limit(size_t newLimit) {
- if (newLimit < 0 || newLimit > cap) {
+ if (newLimit > cap) {
throw IllegalArgumentException("newLimit");
}
lim = newLimit;
Index: src/nameabbreviator.cpp
===================================================================
--- src/nameabbreviator.cpp (Revision 494555)
+++ src/nameabbreviator.cpp (Arbeitskopie)
@@ -221,7 +221,7 @@
PatternAbbreviatorFragment terminalFragment =
fragments[fragments.size() - 1];
- while ((pos < buf.length()) && (pos >= 0)) {
+ while ((pos < buf.length())) {
pos = terminalFragment.abbreviate(buf, pos);
}
}
@@ -265,7 +265,7 @@
int charCount;
LogString::size_type pos = 0;
- while ((pos < trimmed.length()) && (pos >= 0)) {
+ while ((pos < trimmed.length())) {
LogString::size_type ellipsisPos = pos;
if (trimmed[pos] == LOG4CXX_STR('*')) {
Re: Unnecessary comparisons
Posted by Curt Arnold <ca...@apache.org>.
Doesn't look like a problem to me. Brought over from log4j where the
arguments are signed.