You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2003/12/17 03:10:48 UTC

PATCH: obscure bugs in formatting of time/date strings

[[[
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;
 }


Re: PATCH: obscure bugs in formatting of time/date strings

Posted by Julian Foad <ju...@btopenworld.com>.
mark benedetto king wrote:
> On Wed, Dec 17, 2003 at 03:10:48AM +0000, Julian Foad wrote:
> 
>>   Fix the formatting of negative fractional-hour time offsets: an offset
>>     of -75 minutes was written as "-01-15" instead of the correct "-0115".
> 
> I think -01:15 is correct for ISO 8601 extended date-time; the -0115 format
> should only be used for "basic" date-time (even though I incorrectly
> cut-and-pasted the TZI format between the two in my recent RFC on rewriting
> the date parser).

I am talking about the time string used for log messages.

Presently (faking odd time zones):

r8041 | striker | 2003-12-19 09:10:02 +0330 (Fri, 19 Dec 2003) | 5 lines
r8041 | striker | 2003-12-19 09:10:02 -01-15 (Fri, 19 Dec 2003) | 5 lines

Fixed by my patch:

r8041 | striker | 2003-12-19 09:10:02 +0330 (Fri, 19 Dec 2003) | 5 lines
r8041 | striker | 2003-12-19 09:10:02 -0115 (Fri, 19 Dec 2003) | 5 lines

- Julian


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: PATCH: obscure bugs in formatting of time/date strings

Posted by mark benedetto king <mb...@lowlatency.com>.
On Wed, Dec 17, 2003 at 03:10:48AM +0000, Julian Foad wrote:
>    Fix the formatting of negative fractional-hour time offsets: an offset
>      of -75 minutes was written as "-01-15" instead of the correct "-0115".

I think -01:15 is correct for ISO 8601 extended date-time; the -0115 format
should only be used for "basic" date-time (even though I incorrectly
cut-and-pasted the TZI format between the two in my recent RFC on rewriting
the date parser).

--ben


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: PATCH: obscure bugs in formatting of time/date strings

Posted by Julian Foad <ju...@btopenworld.com>.
Sander Roobol wrote:
> On Wed, Dec 17, 2003 at 03:10:48AM +0000, Julian Foad wrote:
> 
>>[[[
>>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.
>>]]]
> 
> This patch hasn't been applied yet. Do you want me to file an issue?

Yes, please - unless anyone can give the thumbs up (or down) right now.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org