The buildbots are failing all over the place after this revision...
On Mon, Sep 6, 2010 at 20:09, <stsp_at_apache.org> wrote:
> 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;
> }
> }
> }
>
>
>
Received on 2010-09-07 06:43:17 CEST