The buildbots are failing all over the place after this revision...
On Mon, Sep 6, 2010 at 20:09, <s...@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; > } > } > } > > >
