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.