This is the mail archive of the ecos-patches@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: language/c/libc/time/current/tests/strptime fails with assertions enabled


On Sat, Aug 18, 2007 at 07:33:48PM +0200, Hans Rosenfeld wrote:
> In this test, strptime() does not assign a value to tm1.tm_yday (it is
> not told to do so), so it stays uninitialized. As far as I know this is
> perfectly legal. The random value of tm_yday causes a CYG_PRECONDITION
> in strftime() fail.
> 
> I think it would be better to move the precondition tests from the
> beginning of strftime() right before the code that does actually use the
> tested members of struct tm, so that strftime() will still work if no
> uninitialized parts of struct tm are used.

Thanks for the patch. I extended it slightly. The assert would not
trigger on the synthetic target. The random value passed is not
sufficiently big enough to case a failure. So i deliberately gave
tm_yday and invalid value. This caused an assert failure as you
described. With your patch everything works fine.

    Thanks
          Andrew
Index: language/c/libc/time/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/time/current/ChangeLog,v
retrieving revision 1.24
diff -u -r1.24 ChangeLog
--- language/c/libc/time/current/ChangeLog	2 Oct 2006 14:17:43 -0000	1.24
+++ language/c/libc/time/current/ChangeLog	15 Sep 2007 14:28:34 -0000
@@ -1,3 +1,15 @@
+2007-09-15  Andrew Lunn  <andrew.lunn@ascom.ch>
+
+	* tests/strptime.c (test): Extend the test so that it triggers the
+	previous bug and shows that the fix works.
+
+2007-08-18  Hans Rosenfeld  <rosenfeld@grumpf.hope-2000.org>
+
+	* src/strftime.cxx: Moved CYG_PRECONDITIONs to do_format() to
+	make strftime() only complain about illegal struct tm contents if
+	these are actually used. Fixes a bug with tests/strptime which
+	would fail because tm->tm_yday was uninitialized.
+	
 2006-10-02  Jonathan Larmour  <jifl@eCosCentric.com>
 
 	* tests/strftime.c (test): Fix %I test.
Index: language/c/libc/time/current/tests/strptime.c
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/time/current/tests/strptime.c,v
retrieving revision 1.2
diff -u -r1.2 strptime.c
--- language/c/libc/time/current/tests/strptime.c	16 Jun 2006 18:46:11 -0000	1.2
+++ language/c/libc/time/current/tests/strptime.c	15 Sep 2007 14:28:34 -0000
@@ -84,7 +84,12 @@
     
     dp = "Fri Jan 24 08:33:14 2003";
     fp = "%a %b %d %H:%M:%S %Y";
-    sp = strptime(dp, fp, &tm1);    
+    sp = strptime(dp, fp, &tm1);
+    
+    // Set an invalid year day. The following converters don't use
+    // this, so it should not cause a problem.
+    tm1.tm_yday = 1000;
+
     CYG_TEST_PASS_FAIL(((sp!=NULL) && (*sp=='\0')), "strptime test #1");
     size = strftime(s, sizeof(s), fp, &tm1);
     CYG_TEST_PASS_FAIL(((size==strlen(dp)) && (my_strcmp(s, dp) == 0)), "strptime test #2");
Index: language/c/libc/time/current/src/strftime.cxx
===================================================================
RCS file: /cvs/ecos/ecos/packages/language/c/libc/time/current/src/strftime.cxx,v
retrieving revision 1.6
diff -u -r1.6 strftime.cxx
--- language/c/libc/time/current/src/strftime.cxx	25 Aug 2006 00:12:41 -0000	1.6
+++ language/c/libc/time/current/src/strftime.cxx	15 Sep 2007 14:28:35 -0000
@@ -85,6 +85,8 @@
 
     switch (fmtchar) {
     case 'a':
+        CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
+                         "timeptr->tm_wday out of range!");
         if (sizeleft<3)
             return -1;
         buf[0] = cyg_libc_time_day_name[timeptr->tm_wday][0];
@@ -92,6 +94,8 @@
         buf[2] = cyg_libc_time_day_name[timeptr->tm_wday][2];
         return 3;
     case 'A':
+        CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
+                         "timeptr->tm_wday out of range!");
         if (sizeleft < cyg_libc_time_day_name_len[timeptr->tm_wday])
             return -1;
         for (i=0; i<cyg_libc_time_day_name_len[timeptr->tm_wday]; ++i)
@@ -102,6 +106,8 @@
         // ** fall through **
 #endif
     case 'b':
+        CYG_PRECONDITION((timeptr->tm_mon >= 0) && (timeptr->tm_mon < 12),
+                         "timeptr->tm_mon out of range!");
         if (sizeleft<3)
             return -1;
         buf[0] = cyg_libc_time_month_name[timeptr->tm_mon][0];
@@ -109,6 +115,8 @@
         buf[2] = cyg_libc_time_month_name[timeptr->tm_mon][2];
         return 3;
     case 'B':
+        CYG_PRECONDITION((timeptr->tm_mon >= 0) && (timeptr->tm_mon < 12),
+                         "timeptr->tm_mon out of range!");
         if (sizeleft < cyg_libc_time_month_name_len[timeptr->tm_mon])
             return -1;
         for (i=0; i<cyg_libc_time_month_name_len[timeptr->tm_mon]; ++i)
@@ -125,6 +133,11 @@
         
         return ((0==i) ? -1 : i);
     case 'd':
+        // Currently I don't check _actual_ numbers of days in each month here
+        // FIXME: No reason why not though
+        CYG_PRECONDITION((timeptr->tm_mday >= 1) && (timeptr->tm_mday < 32),
+                         "timeptr->tm_mday out of range!");
+
         if (sizeleft < 2)
             return -1;
         buf[0] = (timeptr->tm_mday / 10) + '0';
@@ -132,6 +145,10 @@
         return 2;
 #ifdef CYGFUN_LIBC_TIME_SUS_EXTNS
     case 'e':
+        // Currently I don't check _actual_ numbers of days in each month here
+        // FIXME: No reason why not though
+        CYG_PRECONDITION((timeptr->tm_mday >= 1) && (timeptr->tm_mday < 32),
+                         "timeptr->tm_mday out of range!");
         if (sizeleft < 2)
             return -1;
         i = (timeptr->tm_mday / 10);
@@ -140,18 +157,24 @@
         return 2;
 #endif
     case 'H':
+        CYG_PRECONDITION((timeptr->tm_hour >= 0) && (timeptr->tm_hour < 24),
+                         "timeptr->tm_hour out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = (timeptr->tm_hour / 10) + '0';
         buf[1] = (timeptr->tm_hour % 10) + '0';
         return 2;
     case 'I':
+        CYG_PRECONDITION((timeptr->tm_hour >= 0) && (timeptr->tm_hour < 24),
+                         "timeptr->tm_hour out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = (((timeptr->tm_hour%12) ? (timeptr->tm_hour%12) : 12) / 10) + '0';
         buf[1] = (((timeptr->tm_hour%12) ? (timeptr->tm_hour%12) : 12) % 10) + '0';
         return 2;
     case 'j':
+        CYG_PRECONDITION((timeptr->tm_yday >= 0) && (timeptr->tm_yday < 366),
+                         "timeptr->tm_yday out of range!");
         if (sizeleft < 3)
             return -1;
         buf[0] = (timeptr->tm_yday / 100) + '0';
@@ -159,24 +182,32 @@
         buf[2] = (timeptr->tm_yday % 10) + '0';
         return 3;
     case 'm':
+        CYG_PRECONDITION((timeptr->tm_mon >= 0) && (timeptr->tm_mon < 12),
+                         "timeptr->tm_mon out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = ((timeptr->tm_mon+1) / 10) + '0';
         buf[1] = ((timeptr->tm_mon+1) % 10) + '0';
         return 2;
     case 'M':
+        CYG_PRECONDITION((timeptr->tm_min >= 0) && (timeptr->tm_min < 60),
+                         "timeptr->tm_min out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = (timeptr->tm_min / 10) + '0';
         buf[1] = (timeptr->tm_min % 10) + '0';
         return 2;
     case 'p':
+        CYG_PRECONDITION((timeptr->tm_hour >= 0) && (timeptr->tm_hour < 24),
+                         "timeptr->tm_hour out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = (timeptr->tm_hour > 11) ? 'p' : 'a';
         buf[1] = 'm';
         return 2;
     case 'S':
+        CYG_PRECONDITION((timeptr->tm_sec >= 0) && (timeptr->tm_sec < 62),
+                         "timeptr->tm_sec out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = (timeptr->tm_sec / 10) + '0';
@@ -195,6 +226,10 @@
         return ((0==i) ? -1 : i);
 #endif
     case 'U':
+        CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
+                         "timeptr->tm_wday out of range!");
+        CYG_PRECONDITION((timeptr->tm_yday >= 0) && (timeptr->tm_yday < 366),
+                         "timeptr->tm_yday out of range!");
         if (sizeleft < 2)
             return -1;
         i = (timeptr->tm_yday - timeptr->tm_wday + 7) / 7;
@@ -202,10 +237,16 @@
         buf[1] = (i % 10) + '0';
         return 2;
     case 'w':
+        CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
+                         "timeptr->tm_wday out of range!");
         // Don't need to check size - we'll always be called with sizeleft > 0
         buf[0] = timeptr->tm_wday + '0';
         return 1;
     case 'W':
+        CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
+                         "timeptr->tm_wday out of range!");
+        CYG_PRECONDITION((timeptr->tm_yday >= 0) && (timeptr->tm_yday < 366),
+                         "timeptr->tm_yday out of range!");
         if (sizeleft < 2)
             return -1;
         i = (timeptr->tm_yday + ((8-timeptr->tm_wday) % 7)) / 7;
@@ -233,12 +274,18 @@
 
         return (0==i) ? -1 : i;
     case 'y':
+        CYG_PRECONDITION((timeptr->tm_year > -1900) &&
+                         (timeptr->tm_year < 8100),
+                         "timeptr->tm_year out of range!");
         if (sizeleft < 2)
             return -1;
         buf[0] = ((timeptr->tm_year % 100) / 10) + '0';
         buf[1] = ((timeptr->tm_year % 100) % 10) + '0';
         return 2;
     case 'Y':
+        CYG_PRECONDITION((timeptr->tm_year > -1900) &&
+                         (timeptr->tm_year < 8100),
+                         "timeptr->tm_year out of range!");
         if (sizeleft < 4)
             return -1;
         buf[0] = ((1900+timeptr->tm_year) / 1000) + '0';
@@ -274,26 +321,6 @@
                         "timeptr is at address %08x",
                         s, maxsize, format, timeptr);
 
-    CYG_PRECONDITION((timeptr->tm_sec >= 0) && (timeptr->tm_sec < 62),
-                     "timeptr->tm_sec out of range!");
-    CYG_PRECONDITION((timeptr->tm_min >= 0) && (timeptr->tm_min < 60),
-                     "timeptr->tm_min out of range!");
-    CYG_PRECONDITION((timeptr->tm_hour >= 0) && (timeptr->tm_hour < 24),
-                     "timeptr->tm_hour out of range!");
-    // Currently I don't check _actual_ numbers of days in each month here
-    // FIXME: No reason why not though
-    CYG_PRECONDITION((timeptr->tm_mday >= 1) && (timeptr->tm_mday < 32),
-                     "timeptr->tm_mday out of range!");
-    CYG_PRECONDITION((timeptr->tm_mon >= 0) && (timeptr->tm_mon < 12),
-                     "timeptr->tm_mon out of range!");
-    CYG_PRECONDITION((timeptr->tm_wday >= 0) && (timeptr->tm_wday < 7),
-                     "timeptr->tm_wday out of range!");
-    CYG_PRECONDITION((timeptr->tm_yday >= 0) && (timeptr->tm_yday < 366),
-                     "timeptr->tm_yday out of range!");
-    CYG_PRECONDITION((timeptr->tm_year > -1900) &&
-                     (timeptr->tm_year < 8100),
-                     "timeptr->tm_year out of range!");
-
     if (!maxsize) {
         CYG_REPORT_RETVAL(0);
         return 0;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]