Author: stsp
Date: Tue Sep 7 00:09:46 2010
New Revision: 993183
URL: http://svn.apache.org/viewvc?rev=993183&view=rev
Log:
Introduce a new family of functions to parse numbers from strings.
The goal is to replace direct calls to functions like atoi() and apr_atoi64(),
which have very inconvenient error handling to the point where a lot of
our code simply ignores conversion errors and continues operating with
the converted data in potentially undefined state.
The new functions, on the other hand, report conversion errors via the
standard Subversion error handling interface, thereby not allowing callers
to ignore conversion errors.
This commit also switches selected pieces of code over to the new functions.
More conversion to come.
* subversion/include/svn_string.h
(svn_cstring_strtoi64, svn_cstring_atoi64, svn_cstring_atoi,
svn_cstring_strtoui64, svn_cstring_atoui64, svn_cstring_atoui): Declare.
* subversion/libsvn_subr/svn_string.c
(): Include svn_private_config.h for the _() gettext macro.
(svn_cstring_strtoi64, svn_cstring_strtoui64, svn_cstring_atoi64,
svn_cstring_atoi): New.
* subversion/libsvn_ra_serf/serf.c
(dirent_walker): Use svn_cstring_atoi64() instead of apr_atoi64().
* subversion/libsvn_diff/parse-diff.c
(parse_offset): Call svn_cstring_strtoui64() instead of calling
apr_atoi64() and performing manual overflow checking.
* subversion/mod_dav_svn/reports/log.c
(dav_svn__log_report): Use svn_cstring_atoi() instead of atoi() for
parsing CDATA of the "limit" element.
* subversion/mod_dav_svn/reports/replay.c
(dav_svn__replay_report): Use svn_cstring_strtoi64() instead of atoi() for
parsing CDATA of the "send-deltas" element.
* subversion/libsvn_subr/win32_xlate.c
(get_page_id_from_name): Use svn_cstring_atoui() instead of atoi() to parse
the number of a codepage.
* subversion/libsvn_subr/io.c
(svn_io_read_version_file): Use svn_cstring_atoi() instead of atoi().
* subversion/libsvn_subr/hash.c
(svn_hash_read): Use svn_cstring_atoi() instead of atoi().
Modified:
subversion/trunk/subversion/include/svn_string.h
subversion/trunk/subversion/libsvn_diff/parse-diff.c
subversion/trunk/subversion/libsvn_ra_serf/serf.c
subversion/trunk/subversion/libsvn_subr/hash.c
subversion/trunk/subversion/libsvn_subr/io.c
subversion/trunk/subversion/libsvn_subr/svn_string.c
subversion/trunk/subversion/libsvn_subr/win32_xlate.c
subversion/trunk/subversion/mod_dav_svn/reports/log.c
subversion/trunk/subversion/mod_dav_svn/reports/replay.c
Modified: subversion/trunk/subversion/include/svn_string.h
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_string.h?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_string.h (original)
+++ subversion/trunk/subversion/include/svn_string.h Tue Sep 7 00:09:46 2010
@@ -388,6 +388,71 @@ svn_cstring_join(const apr_array_header_
int
svn_cstring_casecmp(const char *str1, const char *str2);
+/**
+ * Parse the C string @a str into a 64 bit number, and return it in @a *n.
+ * Assume that the number is represented in base @a base.
+ * Raise an error if conversion fails (e.g. due to overflow), or if the
+ * converted number is smaller than @a minval or larger than @a maxval.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_strtoi64(apr_int64_t *n, const char *str,
+ apr_int64_t minval, apr_int64_t maxval,
+ int base);
+
+/**
+ * Parse the C string @a str into a 64 bit number, and return it in @a *n.
+ * Assume that the number is represented in base 10.
+ * Raise an error if conversion fails (e.g. due to overflow).
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_atoi64(apr_int64_t *n, const char *str);
+
+/**
+ * Parse the C string @a str into a 32 bit number, and return it in @a *n.
+ * Assume that the number is represented in base 10.
+ * Raise an error if conversion fails (e.g. due to overflow).
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_atoi(int *n, const char *str);
+
+/**
+ * Parse the C string @a str into an unsigned 64 bit number, and return
+ * it in @a *n. Assume that the number is represented in base @a base.
+ * Raise an error if conversion fails (e.g. due to overflow), or if the
+ * converted number is smaller than @a minval or larger than @a maxval.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_strtoui64(apr_uint64_t *n, const char *str,
+ apr_uint64_t minval, apr_uint64_t maxval,
+ int base);
+
+/**
+ * Parse the C string @a str into an unsigned 64 bit number, and return
+ * it in @a *n. Assume that the number is represented in base 10.
+ * Raise an error if conversion fails (e.g. due to overflow).
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_atoui64(apr_uint64_t *n, const char *str);
+
+/**
+ * Parse the C string @a str into an unsigned 32 bit number, and return
+ * it in @a *n. Assume that the number is represented in base 10.
+ * Raise an error if conversion fails (e.g. due to overflow).
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_cstring_atoui(unsigned int *n, const char *str);
/** @} */
Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Tue Sep 7 00:09:46
2010
@@ -28,6 +28,7 @@
#include "svn_error.h"
#include "svn_io.h"
#include "svn_pools.h"
+#include "svn_string.h"
#include "svn_utf.h"
#include "svn_dirent_uri.h"
#include "svn_diff.h"
@@ -119,20 +120,15 @@ svn_diff_hunk_get_trailing_context(const
static svn_boolean_t
parse_offset(svn_linenum_t *offset, const char *number)
{
- apr_int64_t parsed_offset;
+ svn_error_t *err;
- errno = 0; /* apr_atoi64() in APR-0.9 does not always set errno */
- parsed_offset = apr_atoi64(number);
- if (errno == ERANGE || parsed_offset < 0)
- return FALSE;
-
- /* In case we cannot fit 64 bits into an svn_linenum_t,
- * check for overflow. */
- if (sizeof(svn_linenum_t) < sizeof(parsed_offset) &&
- parsed_offset > SVN_LINENUM_MAX_VALUE)
- return FALSE;
-
- *offset = parsed_offset;
+ err = svn_cstring_strtoui64(offset, number, 0, SVN_LINENUM_MAX_VALUE, 10);
+ if (err)
+ {
+ svn_error_clear(err);
+ return FALSE;
+ }
+
return TRUE;
}
Modified: subversion/trunk/subversion/libsvn_ra_serf/serf.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/serf.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/serf.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/serf.c Tue Sep 7 00:09:46 2010
@@ -721,7 +721,7 @@ dirent_walker(void *baton,
}
else if (strcmp(name, "getcontentlength") == 0)
{
- entry->size = apr_atoi64(val->data);
+ SVN_ERR(svn_cstring_atoi64(&entry->size, val->data));
}
else if (strcmp(name, "resourcetype") == 0)
{
Modified: subversion/trunk/subversion/libsvn_subr/hash.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/hash.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/hash.c (original)
+++ subversion/trunk/subversion/libsvn_subr/hash.c Tue Sep 7 00:09:46 2010
@@ -344,11 +344,16 @@ svn_hash_read(apr_hash_t *hash,
}
else if ((buf[0] == 'K') && (buf[1] == ' '))
{
+ size_t keylen;
+ int parsed_len;
+ void *keybuf;
+
/* Get the length of the key */
- size_t keylen = (size_t) atoi(buf + 2);
+ SVN_ERR(svn_cstring_atoi(&parsed_len, buf + 2));
+ keylen = parsed_len;
/* Now read that much into a buffer, + 1 byte for null terminator */
- void *keybuf = apr_palloc(pool, keylen + 1);
+ keybuf = apr_palloc(pool, keylen + 1);
SVN_ERR(svn_io_file_read_full(srcfile,
keybuf, keylen, &num_read, pool));
((char *) keybuf)[keylen] = '\0';
@@ -365,12 +370,15 @@ svn_hash_read(apr_hash_t *hash,
if ((buf[0] == 'V') && (buf[1] == ' '))
{
svn_string_t *value = apr_palloc(pool, sizeof(*value));
+ apr_size_t vallen;
+ void *valbuf;
/* Get the length of the value */
- apr_size_t vallen = atoi(buf + 2);
+ SVN_ERR(svn_cstring_atoi(&parsed_len, buf + 2));
+ vallen = parsed_len;
/* Again, 1 extra byte for the null termination. */
- void *valbuf = apr_palloc(pool, vallen + 1);
+ valbuf = apr_palloc(pool, vallen + 1);
SVN_ERR(svn_io_file_read_full(srcfile,
valbuf, vallen,
&num_read, pool));
Modified: subversion/trunk/subversion/libsvn_subr/io.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/io.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/io.c (original)
+++ subversion/trunk/subversion/libsvn_subr/io.c Tue Sep 7 00:09:46 2010
@@ -3472,7 +3472,7 @@ svn_io_read_version_file(int *version,
}
/* Convert to integer. */
- *version = atoi(buf);
+ SVN_ERR(svn_cstring_atoi(version, buf));
return SVN_NO_ERROR;
}
Modified: subversion/trunk/subversion/libsvn_subr/svn_string.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/svn_string.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/svn_string.c (original)
+++ subversion/trunk/subversion/libsvn_subr/svn_string.c Tue Sep 7 00:09:46
2010
@@ -25,12 +25,15 @@
+#include <apr.h>
+
#include <string.h> /* for memcpy(), memcmp(), strlen() */
#include <apr_lib.h> /* for apr_isspace() */
#include <apr_fnmatch.h>
#include "svn_string.h" /* loads "svn_types.h" and <apr_pools.h> */
#include "svn_ctype.h"
+#include "svn_private_config.h"
/* Our own realloc, since APR doesn't have one. Note: this is a
@@ -605,3 +608,88 @@ svn_cstring_casecmp(const char *str1, co
return cmp;
}
}
+
+svn_error_t *
+svn_cstring_strtoui64(apr_uint64_t *n, const char *str,
+ apr_uint64_t minval, apr_uint64_t maxval,
+ int base)
+{
+ apr_int64_t parsed;
+
+ /* We assume errno is thread-safe. */
+ errno = 0; /* APR-0.9 doesn't always set errno */
+
+ /* ### We're throwing away half the number range here.
+ * ### Maybe implement our own number parser? */
+ parsed = apr_strtoi64(str, NULL, base);
+ if (errno != 0)
+ return svn_error_return(
+ svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("Could not convert '%s' into a number"),
+ str));
+ if (parsed < 0 || parsed < minval || parsed > maxval)
+ return svn_error_return(
+ svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("Number '%s' is out of range"), str));
+ *n = parsed;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_cstring_atoui64(apr_uint64_t *n, const char *str)
+{
+ return svn_error_return(svn_cstring_strtoui64(n, str, 0,
+ APR_UINT64_MAX, 10));
+}
+
+svn_error_t *
+svn_cstring_atoui(unsigned int *n, const char *str)
+{
+ apr_uint64_t parsed;
+
+ SVN_ERR(svn_cstring_strtoui64(&parsed, str, 0, APR_UINT32_MAX, 10));
+ *n = (unsigned int)parsed;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_cstring_strtoi64(apr_int64_t *n, const char *str,
+ apr_int64_t minval, apr_int64_t maxval,
+ int base)
+{
+ apr_int64_t parsed;
+
+ /* We assume errno is thread-safe. */
+ errno = 0; /* APR-0.9 doesn't always set errno */
+
+ parsed = apr_strtoi64(str, NULL, base);
+ if (errno != 0)
+ return svn_error_return(
+ svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("Could not convert '%s' into a number"),
+ str));
+ if (parsed < minval || parsed > maxval)
+ return svn_error_return(
+ svn_error_createf(SVN_ERR_INCORRECT_PARAMS, NULL,
+ _("Number '%s' is out of range"), str));
+ *n = parsed;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_cstring_atoi64(apr_int64_t *n, const char *str)
+{
+ return svn_error_return(svn_cstring_strtoi64(n, str, APR_INT64_MIN,
+ APR_INT64_MAX, 10));
+}
+
+svn_error_t *
+svn_cstring_atoi(int *n, const char *str)
+{
+ apr_int64_t parsed;
+
+ SVN_ERR(svn_cstring_strtoi64(&parsed, str, APR_INT32_MIN,
+ APR_INT32_MAX, 10));
+ *n = (int)parsed;
+ return SVN_NO_ERROR;
+}
Modified: subversion/trunk/subversion/libsvn_subr/win32_xlate.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/win32_xlate.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/win32_xlate.c (original)
+++ subversion/trunk/subversion/libsvn_subr/win32_xlate.c Tue Sep 7 00:09:46
2010
@@ -110,7 +110,7 @@ get_page_id_from_name(UINT *page_id_p, c
if ((page_name[0] == 'c' || page_name[0] == 'C')
&& (page_name[1] == 'p' || page_name[1] == 'P'))
{
- *page_id_p = atoi(page_name + 2);
+ SVN_ERR(svn_cstring_atoui(page_id_p, page_name + 2));
return APR_SUCCESS;
}
Modified: subversion/trunk/subversion/mod_dav_svn/reports/log.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/log.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/log.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/log.c Tue Sep 7 00:09:46
2010
@@ -311,7 +311,15 @@ dav_svn__log_report(const dav_resource *
else if (strcmp(child->name, "end-revision") == 0)
end = SVN_STR_TO_REV(dav_xml_get_cdata(child, resource->pool, 1));
else if (strcmp(child->name, "limit") == 0)
- limit = atoi(dav_xml_get_cdata(child, resource->pool, 1));
+ {
+ serr = svn_cstring_atoi(&limit,
+ dav_xml_get_cdata(child, resource->pool, 1));
+ if (serr)
+ {
+ svn_error_clear(serr);
+ return malformed_element_error("limit", resource->pool);
+ }
+ }
else if (strcmp(child->name, "discover-changed-paths") == 0)
discover_changed_paths = TRUE; /* presence indicates positivity */
else if (strcmp(child->name, "strict-node-history") == 0)
Modified: subversion/trunk/subversion/mod_dav_svn/reports/replay.c
URL:
http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/replay.c?rev=993183&r1=993182&r2=993183&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/replay.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/replay.c Tue Sep 7
00:09:46 2010
@@ -466,10 +466,18 @@ dav_svn__replay_report(const dav_resourc
}
else if (strcmp(child->name, "send-deltas") == 0)
{
+ apr_int64_t parsed_val;
+
cdata = dav_xml_get_cdata(child, resource->pool, 1);
if (! cdata)
return malformed_element_error("send-deltas", resource->pool);
- send_deltas = atoi(cdata);
+ err = svn_cstring_strtoi64(&parsed_val, cdata, 0, 1, 10);
+ if (err)
+ {
+ svn_error_clear(err);
+ return malformed_element_error("send-deltas",
resource->pool);
+ }
+ send_deltas = parsed_val ? TRUE : FALSE;
}
}
}
