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 ts...@apache.org on 2014/02/18 14:56:36 UTC

svn commit: r1569322 - in /incubator/log4cxx/trunk/src: main/cpp/cacheddateformat.cpp test/cpp/helpers/cacheddateformattestcase.cpp

Author: tschoening
Date: Tue Feb 18 13:56:35 2014
New Revision: 1569322

URL: http://svn.apache.org/r1569322
Log:
LOGCXX-425: I added a test for LOGCXX-420 which failed in the former codebase, additionally to the problem with the exceptions. The reason for the exceptions were negativ indices, the calculations for offsets in the magicString introduced in LOGCXX-420 were wrong because the magicString is always 3 chars only.

This and the failing added test for LOGCXX-420 has been fixed, but test 17 of cacheddateformattestcase.cpp is failing now. I will have a look into this afterwards.

Modified:
    incubator/log4cxx/trunk/src/main/cpp/cacheddateformat.cpp
    incubator/log4cxx/trunk/src/test/cpp/helpers/cacheddateformattestcase.cpp

Modified: incubator/log4cxx/trunk/src/main/cpp/cacheddateformat.cpp
URL: http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/main/cpp/cacheddateformat.cpp?rev=1569322&r1=1569321&r2=1569322&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/main/cpp/cacheddateformat.cpp (original)
+++ incubator/log4cxx/trunk/src/main/cpp/cacheddateformat.cpp Tue Feb 18 13:56:35 2014
@@ -132,29 +132,32 @@ int CachedDateFormat::findMillisecondSta
            LogString plusZero;
            formatter->format(plusZero, slotBegin, pool);
 
-           // If the next 1..3 characters match the magic strings, depending on if the currently
+           // If the next 1..3 characters match the magic string, depending on if the currently
            // used millis overlap with the magic string, and the remaining fragments are identical.
+           // Because magic string and currently used millis can overlap, i is not always the index
+           // of the first millis char.
            //
            // LOG4CXX-420:
            // pattern:		%d{yyyy-MM-dd HH:mm:ss,SSS}
            // formatted:	2010-08-12 11:04:50,609
            // plusMagic:	2010-08-12 11:04:50,654
            // plusZero:		2010-08-12 11:04:50,000
+           int possibleRetVal = i - (3 - (formatted.length() - i));
            if (plusZero.length() == formatted.length()
-              && regionMatches(magicString, magicString.length() - (plusMagic.length() - i), plusMagic, i, plusMagic.length() - i)
-              && regionMatches(formattedMillis, formattedMillis.length() - (formatted.length() - i), formatted, i, formatted.length() - i)
-              && regionMatches(zeroString, (sizeof(zeroString)/sizeof(zeroString[0]) - 1) - (plusZero.length() - i), plusZero, i, plusZero.length() - i)
-              && (formatted.length() == i + (formatted.length() - i)
-                 || plusZero.compare(i + (plusZero.length() - i),
-                       LogString::npos, plusMagic, i + (plusMagic.length() - i), LogString::npos) == 0)) {
-              return i - (3 - (formatted.length() - i));
+              && regionMatches(magicString,		0, plusMagic,	possibleRetVal, plusMagic.length()	- possibleRetVal)
+              && regionMatches(formattedMillis,	0, formatted,	possibleRetVal, formatted.length()	- possibleRetVal)
+              && regionMatches(zeroString,		0, plusZero,	possibleRetVal, plusZero.length()	- possibleRetVal)
+              && (formatted.length() == possibleRetVal + (formatted.length() - possibleRetVal)
+                 || plusZero.compare(possibleRetVal + (plusZero.length() - possibleRetVal),
+                       LogString::npos, plusMagic, possibleRetVal + (plusMagic.length() - possibleRetVal), LogString::npos) == 0)) {
+              return possibleRetVal;
            } else {
               return UNRECOGNIZED_MILLISECONDS;
           }
         }
      }
   }
-  return  NO_MILLISECONDS;
+  return NO_MILLISECONDS;
 }
 
 
@@ -237,7 +240,7 @@ int CachedDateFormat::findMillisecondSta
 void CachedDateFormat::millisecondFormat(int millis,
      LogString& buf,
      int offset) {
-     buf[offset] = digits[ millis / 100];
+     buf[offset] = digits[millis / 100];
      buf[offset + 1] = digits[(millis / 10) % 10];
      buf[offset + 2] = digits[millis  % 10];
  }

Modified: incubator/log4cxx/trunk/src/test/cpp/helpers/cacheddateformattestcase.cpp
URL: http://svn.apache.org/viewvc/incubator/log4cxx/trunk/src/test/cpp/helpers/cacheddateformattestcase.cpp?rev=1569322&r1=1569321&r2=1569322&view=diff
==============================================================================
--- incubator/log4cxx/trunk/src/test/cpp/helpers/cacheddateformattestcase.cpp (original)
+++ incubator/log4cxx/trunk/src/test/cpp/helpers/cacheddateformattestcase.cpp Tue Feb 18 13:56:35 2014
@@ -49,7 +49,7 @@ using namespace log4cxx::pattern;
 
 /**
    Unit test {@link CachedDateFormat}.
-   
+
    */
 LOGUNIT_CLASS(CachedDateFormatTestCase)
    {
@@ -140,11 +140,11 @@ LOGUNIT_CLASS(CachedDateFormatTestCase)
      LogString actual;
      gmtFormat.format(actual, jul2, p);
      LOGUNIT_ASSERT_EQUAL((LogString) LOG4CXX_STR("00:00:00,000"), actual);
-     
+
      actual.erase(actual.begin(), actual.end());
      chicagoFormat.format(actual, jul2, p);
      LOGUNIT_ASSERT_EQUAL((LogString) LOG4CXX_STR("19:00:00,000"), actual);
-     
+
      actual.erase(actual.begin(), actual.end());
      gmtFormat.format(actual, jul2, p);
      LOGUNIT_ASSERT_EQUAL((LogString) LOG4CXX_STR("00:00:00,000"), actual);
@@ -450,17 +450,40 @@ void test11() {
  * Check pattern location for ISO8601
  */
 void test12() {
-   DateFormatPtr df = new SimpleDateFormat(LOG4CXX_STR("yyyy-MM-dd HH:mm:ss,SSS"));
-   apr_time_t ticks = 11142L * MICROSECONDS_PER_DAY;
-
+   DateFormatPtr df    = new SimpleDateFormat(LOG4CXX_STR("yyyy-MM-dd HH:mm:ss,SSS"));
+   apr_time_t    ticks = 11142L * MICROSECONDS_PER_DAY;
    Pool p;
-
    LogString formatted;
+
    df->format(formatted, ticks, p);
+   int msStart = CachedDateFormat::findMillisecondStart(ticks, formatted, df, p);
+   LOGUNIT_ASSERT_EQUAL(20, msStart);
 
-   int millisecondStart = CachedDateFormat::findMillisecondStart(ticks,
-       formatted, df, p);
-   LOGUNIT_ASSERT_EQUAL(20, millisecondStart);
+   // Test for for milliseconds overlapping with the magic ones as per LOGCXX-420.
+   apr_time_exp_t c;
+   memset(&c, 0, sizeof(c));
+   c.tm_year = 110;
+   c.tm_mon  = 7;
+   c.tm_mday = 12;
+   c.tm_hour = 9;
+   c.tm_min  = 4;
+   c.tm_sec  = 50;
+   c.tm_usec = 406000;
+
+   LOGUNIT_ASSERT_EQUAL(0, apr_time_exp_gmt_get(&ticks, &c));
+
+   formatted.clear();
+   df->format(formatted, ticks, p);
+   int msStartLogcxx420_406 = CachedDateFormat::findMillisecondStart(ticks, formatted, df, p);
+   LOGUNIT_ASSERT_EQUAL(20, msStartLogcxx420_406);
+
+   c.tm_usec = 609000;
+   LOGUNIT_ASSERT_EQUAL(0, apr_time_exp_gmt_get(&ticks, &c));
+
+   formatted.clear();
+   df->format(formatted, ticks, p);
+   int msStartLogcxx420_609 = CachedDateFormat::findMillisecondStart(ticks, formatted, df, p);
+   LOGUNIT_ASSERT_EQUAL(20, msStartLogcxx420_609);
 }
 
 /**