[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

PATCH: obscure bugs in formatting of time/date strings

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2003-12-17 04:10:48 CET

[[[
Fix obscure bugs in formatting of time/date strings, and tidy the code.

* subversion/include/svn_time.h
  Do not include unnecessary headers.
  Fix a doc string.

* subversion/libsvn_subr/time.c
  Fix white space and a typo.
  (svn_time_to_cstring): Make parameter names match the prototype.
  (svn_time_to_human_cstring):
    Make parameter names match the prototype.
    Fix the formatting of negative fractional-hour time offsets: an offset
      of -75 minutes was written as "-01-15" instead of the correct "-0115".
    Ensure that the string is terminated even if the human-readable part
      cannot be written.
]]]

Here is the diff, ignoring white space changes. (The full diff is attached.)

Index: subversion/include/svn_time.h
===================================================================
--- subversion/include/svn_time.h (revision 8018)
+++ subversion/include/svn_time.h (working copy)
@@ -23,10 +23,8 @@
 #define SVN_TIME_H

 #include <apr_pools.h>
-#include <apr_tables.h>
 #include <apr_time.h>

-#include "svn_string.h"
 #include "svn_error.h"

 #ifdef __cplusplus
@@ -40,7 +38,9 @@
  */
 const char *svn_time_to_cstring (apr_time_t when, apr_pool_t *pool);

-/** Convert @a timestr to an @c apr_time_t @a when, allocated in @a pool. */
+/** Convert @a data to an @c apr_time_t @a when.
+ * Use @a pool for temporary memory allocation.
+ */
 svn_error_t *svn_time_from_cstring (apr_time_t *when, const char *data,
                                     apr_pool_t *pool);

Index: subversion/libsvn_subr/time.c
===================================================================
--- subversion/libsvn_subr/time.c (revision 8018)
+++ subversion/libsvn_subr/time.c (working copy)
@@ -19,6 +19,7 @@

 #include <string.h>
+#include <stdlib.h>
 #include <apr_pools.h>
 #include <apr_time.h>
 #include <apr_strings.h>
@@ -33,7 +34,7 @@
  * "2002-05-07Thh:mm:ss.uuuuuuZ"
  *
  * The format is conformant with ISO-8601 and the date format required
- * by RFC2518 for creationdate. It is a direct converision between
+ * by RFC2518 for creationdate. It is a direct conversion between
  * apr_time_t and a string, so converting to string and back retains
  * the exact value.
  */
@@ -79,7 +80,7 @@

 const char *
-svn_time_to_cstring (apr_time_t t, apr_pool_t *pool)
+svn_time_to_cstring (apr_time_t when, apr_pool_t *pool)
 {
   const char *t_cstr;
   apr_time_exp_t exploded_time;
@@ -94,7 +95,7 @@
      tm_isdst to be not set. We also ignore the weekday and yearday,
      since those are not needed. */

- apr_time_exp_gmt (&exploded_time, t);
+ apr_time_exp_gmt (&exploded_time, when);

   /* It would be nice to use apr_strftime(), but APR doesn't give a
      way to convert back, so we wouldn't be able to share the format
@@ -227,7 +228,7 @@

 const char *
-svn_time_to_human_cstring (apr_time_t t, apr_pool_t *pool)
+svn_time_to_human_cstring (apr_time_t when, apr_pool_t *pool)
 {
   apr_time_exp_t exploded_time;
   apr_size_t len, retlen;
@@ -235,7 +236,7 @@
   char *datestr, *curptr;

   /* Get the time into parts */
- apr_time_exp_lt (&exploded_time, t);
+ apr_time_exp_lt (&exploded_time, when);

   /* Make room for datestring */
   datestr = apr_palloc(pool, SVN_TIME__MAX_LENGTH);
@@ -251,7 +252,7 @@
                       exploded_time.tm_min,
                       exploded_time.tm_sec,
                       exploded_time.tm_gmtoff / (60 * 60),
- (exploded_time.tm_gmtoff / 60) % 60);
+ (abs (exploded_time.tm_gmtoff) / 60) % 60);

   /* If we overfilled the buffer, just return what we got. */
   if(len >= SVN_TIME__MAX_LENGTH)
@@ -268,8 +269,8 @@
                       &exploded_time);

   /* If there was an error, ensure that the string is zero-terminated. */
- if (!ret || retlen == 0)
- curptr = '\0';
+ if (ret || retlen == 0)
+ *curptr = '\0';

   return datestr;
 }

Fix obscure bugs in formatting of time/date strings, and tidy the code.

* subversion/include/svn_time.h
  Do not include unnecessary headers.
  Fix a doc string.

* subversion/libsvn_subr/time.c
  Fix white space and a typo.
  (svn_time_to_cstring): Make parameter names match the prototype.
  (svn_time_to_human_cstring):
    Make parameter names match the prototype.
    Fix the formatting of negative fractional-hour time offsets: an offset
      of -75 minutes was written as "-01-15" instead of the correct "-0115".
    Ensure that the string is terminated even if the human-readable part
      cannot be written.

Index: subversion/include/svn_time.h
===================================================================
--- subversion/include/svn_time.h (revision 8018)
+++ subversion/include/svn_time.h (working copy)
@@ -23,10 +23,8 @@
 #define SVN_TIME_H
 
 #include <apr_pools.h>
-#include <apr_tables.h>
 #include <apr_time.h>
 
-#include "svn_string.h"
 #include "svn_error.h"
 
 #ifdef __cplusplus
@@ -40,7 +38,9 @@
  */
 const char *svn_time_to_cstring (apr_time_t when, apr_pool_t *pool);
 
-/** Convert @a timestr to an @c apr_time_t @a when, allocated in @a pool. */
+/** Convert @a data to an @c apr_time_t @a when.
+ * Use @a pool for temporary memory allocation.
+ */
 svn_error_t *svn_time_from_cstring (apr_time_t *when, const char *data,
                                     apr_pool_t *pool);
 
Index: subversion/libsvn_subr/time.c
===================================================================
--- subversion/libsvn_subr/time.c (revision 8018)
+++ subversion/libsvn_subr/time.c (working copy)
@@ -19,6 +19,7 @@
 
 
 #include <string.h>
+#include <stdlib.h>
 #include <apr_pools.h>
 #include <apr_time.h>
 #include <apr_strings.h>
@@ -33,7 +34,7 @@
  * "2002-05-07Thh:mm:ss.uuuuuuZ"
  *
  * The format is conformant with ISO-8601 and the date format required
- * by RFC2518 for creationdate. It is a direct converision between
+ * by RFC2518 for creationdate. It is a direct conversion between
  * apr_time_t and a string, so converting to string and back retains
  * the exact value.
  */
@@ -79,7 +80,7 @@
 
 
 const char *
-svn_time_to_cstring (apr_time_t t, apr_pool_t *pool)
+svn_time_to_cstring (apr_time_t when, apr_pool_t *pool)
 {
   const char *t_cstr;
   apr_time_exp_t exploded_time;
@@ -94,7 +95,7 @@
      tm_isdst to be not set. We also ignore the weekday and yearday,
      since those are not needed. */
 
- apr_time_exp_gmt (&exploded_time, t);
+ apr_time_exp_gmt (&exploded_time, when);
 
   /* It would be nice to use apr_strftime(), but APR doesn't give a
      way to convert back, so we wouldn't be able to share the format
@@ -144,7 +145,7 @@
 
 
 svn_error_t *
-svn_time_from_cstring(apr_time_t *when, const char *data, apr_pool_t *pool)
+svn_time_from_cstring (apr_time_t *when, const char *data, apr_pool_t *pool)
 {
   apr_time_exp_t exploded_time;
   apr_status_t apr_err;
@@ -154,19 +155,19 @@
   /* Open-code parsing of the new timestamp format, as this
      is a hot path for reading the entries file. This format looks
      like: "2001-08-31T04:24:14.966996Z" */
- exploded_time.tm_year = strtol(c, (char**)&c, 10);
+ exploded_time.tm_year = strtol (c, (char**)&c, 10);
   if (*c++ != '-') goto fail;
- exploded_time.tm_mon = strtol(c, (char**)&c, 10);
+ exploded_time.tm_mon = strtol (c, (char**)&c, 10);
   if (*c++ != '-') goto fail;
- exploded_time.tm_mday = strtol(c, (char**)&c, 10);
+ exploded_time.tm_mday = strtol (c, (char**)&c, 10);
   if (*c++ != 'T') goto fail;
- exploded_time.tm_hour = strtol(c, (char**)&c, 10);
+ exploded_time.tm_hour = strtol (c, (char**)&c, 10);
   if (*c++ != ':') goto fail;
- exploded_time.tm_min = strtol(c, (char**)&c, 10);
+ exploded_time.tm_min = strtol (c, (char**)&c, 10);
   if (*c++ != ':') goto fail;
- exploded_time.tm_sec = strtol(c, (char**)&c, 10);
+ exploded_time.tm_sec = strtol (c, (char**)&c, 10);
   if (*c++ != '.') goto fail;
- exploded_time.tm_usec = strtol(c, (char**)&c, 10);
+ exploded_time.tm_usec = strtol (c, (char**)&c, 10);
   if (*c++ != 'Z') goto fail;
 
   exploded_time.tm_year -= 1900;
@@ -176,30 +177,30 @@
   exploded_time.tm_isdst = 0;
   exploded_time.tm_gmtoff = 0;
 
- apr_err = apr_time_exp_gmt_get(when, &exploded_time);
+ apr_err = apr_time_exp_gmt_get (when, &exploded_time);
   if (apr_err == APR_SUCCESS)
     return SVN_NO_ERROR;
 
- return svn_error_create(SVN_ERR_BAD_DATE, NULL,
- "Date conversion failed.");
+ return svn_error_create (SVN_ERR_BAD_DATE, NULL,
+ "Date conversion failed.");
 
  fail:
   /* Try the compatibility option. This does not need to be fast,
      as this format is no longer generated and the client will convert
      an old-format entries file the first time it reads it. */
- if (sscanf(data,
- old_timestamp_format,
- wday,
- &exploded_time.tm_mday,
- month,
- &exploded_time.tm_year,
- &exploded_time.tm_hour,
- &exploded_time.tm_min,
- &exploded_time.tm_sec,
- &exploded_time.tm_usec,
- &exploded_time.tm_yday,
- &exploded_time.tm_isdst,
- &exploded_time.tm_gmtoff) == 11)
+ if (sscanf (data,
+ old_timestamp_format,
+ wday,
+ &exploded_time.tm_mday,
+ month,
+ &exploded_time.tm_year,
+ &exploded_time.tm_hour,
+ &exploded_time.tm_min,
+ &exploded_time.tm_sec,
+ &exploded_time.tm_usec,
+ &exploded_time.tm_yday,
+ &exploded_time.tm_isdst,
+ &exploded_time.tm_gmtoff) == 11)
     {
       exploded_time.tm_year -= 1900;
       exploded_time.tm_yday -= 1;
@@ -209,7 +210,7 @@
       exploded_time.tm_mon = find_matching_string (month, 12, apr_month_snames);
 
       apr_err = apr_time_exp_gmt_get (when, &exploded_time);
- if(apr_err != APR_SUCCESS)
+ if (apr_err != APR_SUCCESS)
         {
           return svn_error_create (SVN_ERR_BAD_DATE, NULL,
                                    "Date conversion failed.");
@@ -227,7 +228,7 @@
 
 
 const char *
-svn_time_to_human_cstring (apr_time_t t, apr_pool_t *pool)
+svn_time_to_human_cstring (apr_time_t when, apr_pool_t *pool)
 {
   apr_time_exp_t exploded_time;
   apr_size_t len, retlen;
@@ -235,10 +236,10 @@
   char *datestr, *curptr;
 
   /* Get the time into parts */
- apr_time_exp_lt (&exploded_time, t);
+ apr_time_exp_lt (&exploded_time, when);
 
   /* Make room for datestring */
- datestr = apr_palloc(pool, SVN_TIME__MAX_LENGTH);
+ datestr = apr_palloc (pool, SVN_TIME__MAX_LENGTH);
 
   /* Put in machine parseable part */
   len = apr_snprintf (datestr,
@@ -251,10 +252,10 @@
                       exploded_time.tm_min,
                       exploded_time.tm_sec,
                       exploded_time.tm_gmtoff / (60 * 60),
- (exploded_time.tm_gmtoff / 60) % 60);
+ (abs (exploded_time.tm_gmtoff) / 60) % 60);
 
   /* If we overfilled the buffer, just return what we got. */
- if(len >= SVN_TIME__MAX_LENGTH)
+ if (len >= SVN_TIME__MAX_LENGTH)
     return datestr;
 
   /* Calculate offset to the end of the machine parseable part. */
@@ -268,8 +269,8 @@
                       &exploded_time);
   
   /* If there was an error, ensure that the string is zero-terminated. */
- if (!ret || retlen == 0)
- curptr = '\0';
+ if (ret || retlen == 0)
+ *curptr = '\0';
 
   return datestr;
 }

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 17 04:11:03 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.