Hi subversion devlopers,
I noticed the use of "atoi" in some places followed
by an apr-malloc on the returnvalue without doing any
checks on the returnvalue first.
I'd prefer the use of a string-to-int function with
better error and range checking
(since cases like out-of-range numbers "374623874682734"
invalid characters "+23", "16.2", "err" just
return 0 or other unexpected values with atoi)
======
for example in the new fsfs.c :
>>
size_t keylen = (size_t) atoi (stringbuf->data + 2);
/* Now read that much into a buffer, + 1 byte for null terminator */
void *keybuf = apr_palloc (pool, keylen + 1);
SVN_ERR (svn_stream_read (stream, keybuf, &keylen));
((char *) keybuf)[keylen] = '\0';
<<
for example if "-1" would be returned from atoi then the '\0'
could go into dead memory.
how do apr_palloc(0) and apr_palloc(-2) react?
some similar atoi based alloc code exists in 'load.c' and 'hash.c'(2x)
and maybe other places too.
======
also 'parse_url' from client.c is using atoi on the portnumber
and leads to strange errors if a typo is using characters
where the port number is expected, like in
svn checkout http://some.url.at:X/repos/file
svn checkout http://some.url.at:0/repos/file
svn checkout http://some.url.at:80XYZ/repos/file
svn checkout http://some.url.at:/repos/file
which creates a buggy working copy.
=======
And maybe you find this compiler output (below) helpful too.
- but its nothing serious.
MSVC 7.1 (X86) Build of trunk 9593
=========================
* ...\subversion\libsvn_repos\reporter.c(136) : warning C4244:
'function' : conversion from 'apr_uint64_t' to 'apr_size_t', possible
loss of data
* ...\subversion\libsvn_repos\reporter.c(137) : warning C4244:
'function' : conversion from 'apr_uint64_t' to 'apr_size_t', possible
loss of data
* ...\subversion\libsvn_repos\reporter.c(153) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'svn_revnum_t', possible loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(509) : warning C4244:
'function' : conversion from 'apr_uint64_t' to 'apr_size_t', possible
loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(510) : warning C4244:
'function' : conversion from 'apr_uint64_t' to 'apr_size_t', possible
loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(515) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(599) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'svn_revnum_t', possible loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(738) : warning C4244:
'function' : conversion from 'apr_uint64_t' to 'apr_status_t', possible
loss of data
* ...\subversion\libsvn_ra_svn\marshal.c(740) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'long', possible loss of data
* ...\subversion\libsvn_ra_svn\client.c(454) : warning C4090:
'function' : different 'const' qualifiers
* ...\subversion\libsvn_ra_svn\client.c(569) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'int', possible loss of data
* ...\subversion\libsvn_client\blame.c(237) : warning C4101:
'change' : unreferenced local variable
* ...\subversion\libsvn_delta\text_delta.c(335) : warning C4244:
'function' : conversion from 'svn_filesize_t' to 'apr_size_t', possible
loss of data
* ...\subversion\libsvn_diff\diff_file.c(149) : warning C4244:
'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss
of data
* ...\subversion\libsvn_diff\diff_file.c(163) : warning C4244:
'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss
of data
* ...\subversion\libsvn_diff\diff_file.c(165) : warning C4244:
'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss
of data
* ...\subversion\libsvn_diff\diff_file.c(202) : warning C4244: '=' :
conversion from 'apr_off_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_diff\diff_file.c(256) : warning C4244: '=' :
conversion from 'apr_off_t' to 'int', possible loss of data
* ...\subversion\libsvn_diff\diff_file.c(305) : warning C4244: '=' :
conversion from 'apr_off_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_diff\diff_file.c(397) : warning C4244:
'function' : conversion from 'apr_off_t' to 'apr_size_t', possible loss
of data
* ...\subversion\libsvn_diff\diff_file.c(408) : warning C4244:
'function' : conversion from 'apr_off_t' to 'size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(592) : warning C4244: '=' :
conversion from 'apr_int64_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(599) : warning C4244: '=' :
conversion from 'apr_int64_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(997) : warning C4244: '=' :
conversion from 'apr_int64_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(1441) : warning C4244: '=' :
conversion from 'apr_off_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(2949) : warning C4244: '=' :
conversion from 'apr_off_t' to 'apr_size_t', possible loss of data
* ...\subversion\libsvn_fs_fs\fs_fs.c(2952) : warning C4244: '=' :
conversion from 'svn_filesize_t' to 'apr_size_t', possible loss of data
* ...\subversion\svndumpfilter\main.c(54) : warning C4042: 'open_fn'
: has bad storage class
* ...\subversion\svnserve\serve.c(1085) : warning C4244: '=' :
conversion from 'apr_uint64_t' to 'int', possible loss of data
* ...\subversion\svnadmin\main.c(64) : warning C4042: 'open_fn' :
has bad storage class
=======
:-)
c.a.t.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat May 1 18:05:44 2004