You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nuutti Kotivuori <na...@iki.fi> on 2002/06/16 21:20:22 UTC

[PATCH] Make svn_time_from_nts return an error.

I'm posting this here for a quick review - and to hopefully get a hint
on what should I return as an error code and message.

All the patch does is makes svn_time_from_nts return svn_error_t * -
and then fixing up the places that use it. Should break nothing and
should be goodness.

I'll be committing this, after the fixes, whenever I get around to do
it.

Log message:

Make svn_time_from_nts return an error instead of the value
directly. Also a minor fix to keep svn_time_from_nts from crashing if
bad dates are given in.

* subversion/include/svn_time.h
(svn_time_from_nts): Changed interface to return an error.

* subversion/libsvn_subr/time.c
(svn_time_from_nts): Adapted to return an error.
(find_matching_string): Added size argument for max size. No longer
crashes if a string cannot be found.

* subversion/libsvn_repos/rev_hunt.c
(get_time): Adapt for svn_time_from_nts returning an error.

* subversion/tests/libsvn_subr/time-test.c (test_time_from_nts): Ditto.
(test_time_from_nts_old): Ditto.
(test_time_invariant): Ditto.

* subversion/tests/libsvn_fs/fs-test.c (commit_date): Ditto.

* subversion/svnlook/main.c (do_date): Ditto.

* subversion/libsvn_wc/update_editor.c (change_dir_prop): Ditto.

* subversion/libsvn_wc/entries.c (svn_wc__atts_to_entry): Ditto.

* subversion/libsvn_wc/questions.c (svn_wc__timestamps_equal_p): Ditto.
(svn_wc__timestamps_equal_p): Commented out time to string and back
conversion.

Patch:
Index: ./subversion/include/svn_time.h
===================================================================
--- ./subversion/include/svn_time.h
+++ ./subversion/include/svn_time.h	Sun Jun  9 20:24:55 2002
@@ -37,7 +37,8 @@
 const char *svn_time_to_nts (apr_time_t when, apr_pool_t *pool);
 
 /* Convert TIMESTR to an apr_time_t. */
-apr_time_t svn_time_from_nts (const char *timestr);
+svn_error_t *svn_time_from_nts(apr_time_t *when, const char *data,
+                               apr_pool_t *pool);
 
 /* Needed by getdate.y parser */
 struct getdate_time {
Index: ./subversion/libsvn_wc/entries.c
===================================================================
--- ./subversion/libsvn_wc/entries.c
+++ ./subversion/libsvn_wc/entries.c	Sat Jun 15 00:01:53 2002
@@ -340,7 +340,7 @@
                it.  */
           }
         else
-          entry->text_time = svn_time_from_nts (text_timestr);
+          SVN_ERR (svn_time_from_nts (&entry->text_time, text_timestr, pool));
         
         *modify_flags |= SVN_WC__ENTRY_MODIFY_TEXT_TIME;
       }
@@ -357,7 +357,7 @@
                it.  */
           }
         else
-          entry->prop_time = svn_time_from_nts (prop_timestr);
+          SVN_ERR (svn_time_from_nts (&entry->prop_time, prop_timestr, pool));
         
         *modify_flags |= SVN_WC__ENTRY_MODIFY_PROP_TIME;
       }
@@ -379,7 +379,7 @@
                                 APR_HASH_KEY_STRING);
     if (cmt_datestr)
       {
-        entry->cmt_date = svn_time_from_nts (cmt_datestr);
+        SVN_ERR (svn_time_from_nts (&entry->cmt_date, cmt_datestr, pool));
         *modify_flags |= SVN_WC__ENTRY_MODIFY_CMT_DATE;
       }
     else
Index: ./subversion/libsvn_wc/update_editor.c
===================================================================
--- ./subversion/libsvn_wc/update_editor.c
+++ ./subversion/libsvn_wc/update_editor.c	Sat Jun 15 00:01:56 2002
@@ -744,7 +744,7 @@
         }
       else if ((! strcmp (name, SVN_PROP_ENTRY_COMMITTED_DATE)) && value)
         {
-          entry.cmt_date = svn_time_from_nts (value->data);
+          SVN_ERR (svn_time_from_nts (&entry.cmt_date, value->data, pool));
           modify_flags = SVN_WC__ENTRY_MODIFY_CMT_DATE;
         }
       else if ((! strcmp (name, SVN_PROP_ENTRY_LAST_AUTHOR)) && value)
Index: ./subversion/libsvn_wc/questions.c
===================================================================
--- ./subversion/libsvn_wc/questions.c
+++ ./subversion/libsvn_wc/questions.c	Sun Jun 16 22:41:52 2002
@@ -182,8 +182,15 @@
   {
     /* Put the disk timestamp through a string conversion, so it's
        at the same resolution as entry timestamps. */
+    /* This string conversion here may be goodness, but it does
+       nothing currently _and_ it is somewhat expensive _and_ it eats
+       memory _and_ it is tested for in the regression tests. But I
+       will only comment it out because I do not possess the guts to
+       remove it altogether. */
+    /*
     const char *tstr = svn_time_to_nts (wfile_time, pool);
-    wfile_time = svn_time_from_nts (tstr);
+    SVN_ERR (svn_time_from_nts (&wfile_time, tstr, pool));
+    */
   }
   
   if (wfile_time == entrytime)
Index: ./subversion/libsvn_subr/time.c
===================================================================
--- ./subversion/libsvn_subr/time.c
+++ ./subversion/libsvn_subr/time.c	Sun Jun 16 23:13:34 2002
@@ -110,11 +110,11 @@
 
 
 static int
-find_matching_string (char *str, const char strings[][4])
+find_matching_string (char *str, size_t size, const char strings[][4])
 {
   int i;
 
-  for (i = 0; ; i++)
+  for (i = 0; i < size; i++)
     if (strings[i] && (strcmp (str, strings[i]) == 0))
       return i;
 
@@ -122,12 +122,12 @@
 }
 
 
-apr_time_t
-svn_time_from_nts (const char *data)
+svn_error_t *
+svn_time_from_nts(apr_time_t *when, const char *data, apr_pool_t *pool)
 {
   apr_time_exp_t exploded_time;
+  apr_status_t apr_err;
   char wday[4], month[4];
-  apr_time_t when;
 
   /* First try the new timestamp format. */
   if (sscanf (data,
@@ -146,8 +146,16 @@
       exploded_time.tm_yday = 0;
       exploded_time.tm_isdst = 0;
       exploded_time.tm_gmtoff = 0;
-
-      apr_implode_gmt (&when, &exploded_time);
+      
+      apr_err = apr_implode_gmt (when, &exploded_time);
+      if(apr_err != APR_SUCCESS)
+        {
+          /* ### todo: return a proper error. */
+          return svn_error_createf (SVN_ERR_GENERAL, apr_err, NULL, pool,
+                                    "Date conversion failed.");
+        }
+      
+      return SVN_NO_ERROR;
     }
   /* Then try the compatibility option. */
   else if (sscanf (data,
@@ -166,21 +174,28 @@
     {
       exploded_time.tm_year -= 1900;
       exploded_time.tm_yday -= 1;
-      exploded_time.tm_wday = find_matching_string (wday, apr_day_snames);
-      exploded_time.tm_mon = find_matching_string (month, apr_month_snames);
+      /* Using hard coded limits for the arrays - they are going away
+         soon in any case. */
+      exploded_time.tm_wday = find_matching_string (wday, 7, apr_day_snames);
+      exploded_time.tm_mon = find_matching_string (month, 12, apr_month_snames);
+
+      apr_err = apr_implode_gmt (when, &exploded_time);
+      if(apr_err != APR_SUCCESS)
+        {
+          /* ### todo: return a proper error. */
+          return svn_error_createf (SVN_ERR_GENERAL, apr_err, NULL, pool,
+                                    "Date conversion failed.");
+        }
 
-      apr_implode_gmt (&when, &exploded_time);
+      return SVN_NO_ERROR;
     }
   /* Timestamp is something we do not recognize. */
   else
     {
-      /* XXX: fix this function to return real error codes and all the
-         places that use it. */
-      /* Better zero than something random. */
-      when = 0;
+      /* ### todo: return a proper error. */
+      return svn_error_createf(SVN_ERR_GENERAL, 0, NULL, pool,
+                               "Could not parse date.");
     }
-
-  return when;
 }
 
 
Index: ./subversion/svnlook/main.c
===================================================================
--- ./subversion/svnlook/main.c
+++ ./subversion/svnlook/main.c	Sun Jun 16 13:17:33 2002
@@ -745,8 +745,8 @@
           apr_time_exp_t extime;
           apr_time_t aprtime;
           apr_status_t apr_err;
-              
-          aprtime = svn_time_from_nts (prop_value->data);
+          
+          SVN_ERR (svn_time_from_nts (&aprtime, prop_value->data, pool));
           apr_err = apr_time_exp_tz (&extime, aprtime, 0);
           if (apr_err)
             return svn_error_create (apr_err, 0, NULL, pool,
Index: ./subversion/tests/libsvn_fs/fs-test.c
===================================================================
--- ./subversion/tests/libsvn_fs/fs-test.c
+++ ./subversion/tests/libsvn_fs/fs-test.c	Sat Jun 15 00:02:19 2002
@@ -3661,7 +3661,7 @@
       (SVN_ERR_FS_GENERAL, 0, NULL, pool,
        "failed to get datestamp of committed revision");
 
-  at_commit = svn_time_from_nts (datestamp->data);
+  SVN_ERR (svn_time_from_nts (&at_commit, datestamp->data, pool));
 
   if (at_commit < before_commit)
     return svn_error_create
Index: ./subversion/tests/libsvn_subr/time-test.c
===================================================================
--- ./subversion/tests/libsvn_subr/time-test.c
+++ ./subversion/tests/libsvn_subr/time-test.c	Sun Jun  9 20:45:39 2002
@@ -71,7 +71,7 @@
   if (msg_only)
     return SVN_NO_ERROR;
 
-  timestamp = svn_time_from_nts(test_timestring);
+  SVN_ERR (svn_time_from_nts (&timestamp, test_timestring, pool));
 
   if (timestamp != test_timestamp)
     {
@@ -98,7 +98,7 @@
   if (msg_only)
     return SVN_NO_ERROR;
 
-  timestamp = svn_time_from_nts(test_old_timestring);
+  SVN_ERR (svn_time_from_nts (&timestamp, test_old_timestring, pool));
 
   if (timestamp != test_timestamp)
     {
@@ -128,7 +128,7 @@
     return SVN_NO_ERROR;
 
   timestring = svn_time_to_nts(current_timestamp,pool);
-  timestamp = svn_time_from_nts(timestring);
+  SVN_ERR (svn_time_from_nts (&timestamp, timestring, pool));
 
   if (timestamp != current_timestamp)
     {
Index: ./subversion/libsvn_repos/rev_hunt.c
===================================================================
--- ./subversion/libsvn_repos/rev_hunt.c
+++ ./subversion/libsvn_repos/rev_hunt.c	Sun Jun  9 20:30:47 2002
@@ -58,7 +58,7 @@
       (SVN_ERR_FS_GENERAL, 0, NULL, pool,
        "failed to find tm on revision %" SVN_REVNUM_T_FMT, rev);
 
-  *tm = svn_time_from_nts (date_str->data);
+  SVN_ERR (svn_time_from_nts (tm, date_str->data, pool));
 
   return SVN_NO_ERROR;
 }


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