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 (×tamp, 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 (×tamp, 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 (×tamp, 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