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

[PATCH] Make svn_time_from_nts return an error.

From: Nuutti Kotivuori <naked_at_iki.fi>
Date: 2002-06-16 23:20:22 CEST

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
Received on Sun Jun 16 23:23:14 2002

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