You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2002/04/23 13:12:37 UTC
[PATCH] VC warnings
Hi,
Got lots of warnings on the windows build. This takes care
of them. Not sure we want to typecast ourselves into oblivion
though.
This patch deliberately has no log msg (each cast needs review).
Thoughts?
Sander
Index: ./subversion/libsvn_fs/strings-table.c
===================================================================
--- ./subversion/libsvn_fs/strings-table.c
+++ ./subversion/libsvn_fs/strings-table.c Tue Apr 23 13:18:13 2002
@@ -218,7 +218,7 @@
svn_fs__clear_dbt (&result);
result.data = buf;
result.ulen = *len;
- result.doff = offset;
+ result.doff = (u_int32_t)offset;
result.dlen = *len;
result.flags |= (DB_DBT_USERMEM | DB_DBT_PARTIAL);
db_err = cursor->c_get (cursor, &query, &result, DB_CURRENT);
Index: ./subversion/libsvn_fs/reps-strings.c
===================================================================
--- ./subversion/libsvn_fs/reps-strings.c
+++ ./subversion/libsvn_fs/reps-strings.c Tue Apr 23 13:18:13 2002
@@ -445,12 +445,13 @@
algorithm, and is what has to go when we have a
true delta combiner. */
SVN_ERR (rep_read_range (wb->fs, wb->base_rep, sbuf,
- window->sview_offset, &slen,
+ (apr_size_t)window->sview_offset,
+ &slen,
wb->trail));
src_read = 1;
}
- memcpy (tbuf + len_read, sbuf + op->offset, op->length);
- len_read += op->length;
+ memcpy (tbuf + len_read, sbuf + op->offset, (apr_size_t)op->length);
+ len_read += (apr_size_t)op->length;
}
break;
@@ -459,7 +460,7 @@
/* This could be done in bigger blocks, at the expense
of some more complexity. */
int t;
- for (t = op->offset; t < op->offset + op->length; t++)
+ for (t = (int)op->offset; t < op->offset + op->length; t++)
tbuf[len_read++] = tbuf[t];
}
break;
@@ -468,8 +469,8 @@
{
memcpy (tbuf + len_read,
window->new_data->data + op->offset,
- op->length);
- len_read += op->length;
+ (apr_size_t)op->length);
+ len_read += (apr_size_t)op->length;
}
break;
Index: ./subversion/libsvn_wc/props.c
===================================================================
--- ./subversion/libsvn_wc/props.c
+++ ./subversion/libsvn_wc/props.c Tue Apr 23 13:10:06 2002
@@ -659,19 +659,19 @@
that each .svn subdir remains separable when executing run_log(). */
str = strstr (base_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
len = base_prop_tmp_path->data + base_prop_tmp_path->len - str;
- tmp_prop_base = svn_stringbuf_ncreate (str, len, pool);
+ tmp_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
str = strstr (base_propfile_path->data, SVN_WC_ADM_DIR_NAME);
len = base_propfile_path->data + base_propfile_path->len - str;
- real_prop_base = svn_stringbuf_ncreate (str, len, pool);
+ real_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
str = strstr (local_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
len = local_prop_tmp_path->data + local_prop_tmp_path->len - str;
- tmp_props = svn_stringbuf_ncreate (str, len, pool);
+ tmp_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
str = strstr (local_propfile_path->data, SVN_WC_ADM_DIR_NAME);
len = local_propfile_path->data + local_propfile_path->len - str;
- real_props = svn_stringbuf_ncreate (str, len, pool);
+ real_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
/* Write log entry to move pristine tmp copy to real pristine area. */
svn_xml_make_open_tag (entry_accum,
Index: ./subversion/libsvn_wc/log.c
===================================================================
--- ./subversion/libsvn_wc/log.c
+++ ./subversion/libsvn_wc/log.c Tue Apr 23 13:24:19 2002
@@ -1095,7 +1095,7 @@
apr_hash_this (hi, &key, &keylen, &val);
entry = val;
- if ((keylen == strlen (SVN_WC_ENTRY_THIS_DIR))
+ if ((keylen == (apr_ssize_t)strlen (SVN_WC_ENTRY_THIS_DIR))
&& (strcmp ((char *) key, SVN_WC_ENTRY_THIS_DIR) == 0))
is_this_dir = TRUE;
Index: ./subversion/clients/cmdline/util.c
===================================================================
--- ./subversion/clients/cmdline/util.c
+++ ./subversion/clients/cmdline/util.c Tue Apr 23 13:24:19 2002
@@ -463,10 +463,10 @@
else
{
/* Read the entire file into memory. */
- svn_stringbuf_ensure (new_contents, finfo_after.size + 1);
+ svn_stringbuf_ensure (new_contents, (apr_size_t)(finfo_after.size + 1));
apr_err = apr_file_read_full (tmp_file,
new_contents->data,
- finfo_after.size,
+ (apr_size_t)finfo_after.size,
&(new_contents->len));
new_contents->data[new_contents->len] = 0;
Index: ./subversion/libsvn_delta/svndiff.c
===================================================================
--- ./subversion/libsvn_delta/svndiff.c
+++ ./subversion/libsvn_delta/svndiff.c Tue Apr 23 13:24:19 2002
@@ -80,7 +80,7 @@
while (--n >= 0)
{
cont = ((n > 0) ? 0x1 : 0x0) << 7;
- *p++ = ((val >> (n * 7)) & 0x7f) | cont;
+ *p++ = (char)(((val >> (n * 7)) & 0x7f) | cont);
}
return p;
@@ -341,10 +341,10 @@
case svn_txdelta_new:
if (op.length > new_len - npos)
return -1;
- npos += op.length;
+ npos += (apr_size_t)op.length;
break;
}
- tpos += op.length;
+ tpos += (apr_size_t)op.length;
if (tpos < 0)
return -1;
n++;
@@ -415,22 +415,22 @@
p = decode_int (&val, p, end);
if (p == NULL)
return SVN_NO_ERROR;
- sview_len = val;
+ sview_len = (apr_size_t)val;
p = decode_int (&val, p, end);
if (p == NULL)
return SVN_NO_ERROR;
- tview_len = val;
+ tview_len = (apr_size_t)val;
p = decode_int (&val, p, end);
if (p == NULL)
return SVN_NO_ERROR;
- inslen = val;
+ inslen = (apr_size_t)val;
p = decode_int (&val, p, end);
if (p == NULL)
return SVN_NO_ERROR;
- newlen = val;
+ newlen = (apr_size_t)val;
/* Check for integer overflow (don't want to let the input trick
us into invalid pointer games using negative numbers). */
@@ -477,7 +477,7 @@
if (op->action_code == svn_txdelta_new)
{
op->offset = npos;
- npos += op->length;
+ npos += (apr_size_t)op->length;
}
}
window.num_ops = ninst;
Index: ./subversion/libsvn_delta/text_delta.c
===================================================================
--- ./subversion/libsvn_delta/text_delta.c
+++ ./subversion/libsvn_delta/text_delta.c Tue Apr 23 13:18:46 2002
@@ -155,7 +155,7 @@
op->action_code = opcode;
op->offset = bob->new_data->len;
op->length = length;
- svn_stringbuf_appendbytes (bob->new_data, new_data, length);
+ svn_stringbuf_appendbytes (bob->new_data, new_data, (apr_size_t)length);
break;
default:
assert (!"unknown delta op.");
@@ -351,8 +351,8 @@
case svn_txdelta_source:
/* Copy from source area. */
assert (op->offset + op->length <= window->sview_len);
- memcpy (tbuf + tpos, sbuf + op->offset, op->length);
- tpos += op->length;
+ memcpy (tbuf + tpos, sbuf + op->offset, (apr_size_t)op->length);
+ tpos += (apr_size_t)op->length;
break;
case svn_txdelta_target:
@@ -361,7 +361,7 @@
and target copies are allowed to overlap to generate
repeated data. */
assert (op->offset < tpos);
- for (i = op->offset; i < op->offset + op->length; i++)
+ for (i = (apr_size_t)op->offset; i < op->offset + op->length; i++)
tbuf[tpos++] = tbuf[i];
break;
@@ -370,8 +370,8 @@
assert (op->offset + op->length <= window->new_data->len);
memcpy (tbuf + tpos,
window->new_data->data + op->offset,
- op->length);
- tpos += op->length;
+ (apr_size_t)op->length);
+ tpos += (apr_size_t)op->length;
break;
default:
@@ -421,7 +421,7 @@
* overlap to the beginning of the new buffer. */
if (ab->sbuf_offset + ab->sbuf_len > window->sview_offset)
{
- apr_size_t start = window->sview_offset - ab->sbuf_offset;
+ apr_size_t start = (apr_size_t)(window->sview_offset - ab->sbuf_offset);
memmove (ab->sbuf, old_sbuf + start, ab->sbuf_len - start);
ab->sbuf_len -= start;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Apr 24, 2002 at 03:13:44AM +0200, Sander Striker wrote:
>...
> While avoiding the svn_filesize_t, this gets rid of some warnings. Also
> it won't conflict with when svn_filesize_t is introduced (I hope ;).
+1
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] VC warnings
Posted by Sander Striker <st...@apache.org>.
> From: Greg Stein [mailto:gstein@lyra.org]
> Sent: 23 April 2002 19:58
[...]
> Some of these relate to the issue I filed about adding 'svn_filesize_t'.
> Subversion, as an entire system, should be defined to work with 64-bit-sized
> files. At least from the API sense. Depending upon the local platform, or
> the backend of the FS, or Apache limitations, we might have restrictions
> here and there which will drop it. But the *definition* should be 64-bit
> throughout.
While avoiding the svn_filesize_t, this gets rid of some warnings. Also
it won't conflict with when svn_filesize_t is introduced (I hope ;).
Sander
---
Log:
A 'get rid of warnings' change. Inspired by Greg Stein.
* subversion/include/svn_delta.h
(svn_txdelta_op_t): Change type of 'offset'
and 'length' from apr_off_t to apr_size_t.
* subversion/libsvn_fs/strings-table.c
(string_read): Cast 'offset' to the type used
by Berkeley DB.
* subversion/libsvn_fs/reps-strings.c
(window_handler): Change type of 't' from
int to apr_size_t.
* subversion/libsvn_wc/props.c
(svn_wc__merge_prop_diffs): Change type of
'len' from apr_off_t to apr_size_t. Also
add a typecast.
* subversion/libsvn_wc/log.c
(svn_wc_cleanup): Use Gregs Real Man solution
to set 'is_this_dir'.
* subversion/clients/cmdline/util.c
(svn_cl__edit_externally): Add typecasts.
* subversion/libsvn_delta/svndiff.c
(encode_int): Add typecast.
* subversion/libsvn_delta/text_delta.c
(apply_window): Add typecasts.
Index: ./subversion/include/svn_delta.h
===================================================================
--- ./subversion/include/svn_delta.h
+++ ./subversion/include/svn_delta.h Wed Apr 24 02:17:48 2002
@@ -112,8 +112,8 @@
/* A single text delta instruction. */
typedef struct svn_txdelta_op_t {
enum svn_delta_action action_code;
- apr_off_t offset;
- apr_off_t length;
+ apr_size_t offset;
+ apr_size_t length;
} svn_txdelta_op_t;
Index: ./subversion/libsvn_fs/strings-table.c
===================================================================
--- ./subversion/libsvn_fs/strings-table.c
+++ ./subversion/libsvn_fs/strings-table.c Wed Apr 24 02:11:36 2002
@@ -218,7 +218,7 @@
svn_fs__clear_dbt (&result);
result.data = buf;
result.ulen = *len;
- result.doff = offset;
+ result.doff = (u_int32_t)offset;
result.dlen = *len;
result.flags |= (DB_DBT_USERMEM | DB_DBT_PARTIAL);
db_err = cursor->c_get (cursor, &query, &result, DB_CURRENT);
Index: ./subversion/libsvn_fs/reps-strings.c
===================================================================
--- ./subversion/libsvn_fs/reps-strings.c
+++ ./subversion/libsvn_fs/reps-strings.c Wed Apr 24 02:23:10 2002
@@ -458,7 +458,7 @@
{
/* This could be done in bigger blocks, at the expense
of some more complexity. */
- int t;
+ apr_size_t t;
for (t = op->offset; t < op->offset + op->length; t++)
tbuf[len_read++] = tbuf[t];
}
Index: ./subversion/libsvn_wc/props.c
===================================================================
--- ./subversion/libsvn_wc/props.c
+++ ./subversion/libsvn_wc/props.c Wed Apr 24 02:43:34 2002
@@ -452,7 +452,7 @@
int i;
svn_boolean_t is_dir;
const char * str;
- apr_off_t len;
+ apr_size_t len;
/* Zillions of pathnames to compute! yeargh! */
svn_stringbuf_t *base_propfile_path, *local_propfile_path;
@@ -658,7 +658,7 @@
paths are RELATIVE pathnames (each beginning with ".svn/"), so
that each .svn subdir remains separable when executing run_log(). */
str = strstr (base_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
- len = base_prop_tmp_path->data + base_prop_tmp_path->len - str;
+ len = (apr_size_t)(base_prop_tmp_path->data - str) + base_prop_tmp_path->len;
tmp_prop_base = svn_stringbuf_ncreate (str, len, pool);
str = strstr (base_propfile_path->data, SVN_WC_ADM_DIR_NAME);
Index: ./subversion/libsvn_wc/log.c
===================================================================
--- ./subversion/libsvn_wc/log.c
+++ ./subversion/libsvn_wc/log.c Wed Apr 24 02:33:56 2002
@@ -1090,14 +1090,17 @@
apr_ssize_t keylen;
void *val;
svn_wc_entry_t *entry;
- svn_boolean_t is_this_dir = FALSE;
+ svn_boolean_t is_this_dir;
apr_hash_this (hi, &key, &keylen, &val);
entry = val;
- if ((keylen == strlen (SVN_WC_ENTRY_THIS_DIR))
- && (strcmp ((char *) key, SVN_WC_ENTRY_THIS_DIR) == 0))
- is_this_dir = TRUE;
+#define KLEN (sizeof(SVN_WC_ENTRY_THIS_DIR) - 1)
+
+ is_this_dir = keylen == KLEN
+ && memcmp(key, SVN_WC_ENTRY_THIS_DIR, KLEN) == 0;
+
+#undef KLEN
if ((entry->kind == svn_node_dir) && (! is_this_dir))
{
Index: ./subversion/clients/cmdline/util.c
===================================================================
--- ./subversion/clients/cmdline/util.c
+++ ./subversion/clients/cmdline/util.c Wed Apr 24 02:36:09 2002
@@ -463,10 +463,11 @@
else
{
/* Read the entire file into memory. */
- svn_stringbuf_ensure (new_contents, finfo_after.size + 1);
+ svn_stringbuf_ensure (new_contents,
+ (apr_size_t)(finfo_after.size + 1));
apr_err = apr_file_read_full (tmp_file,
new_contents->data,
- finfo_after.size,
+ (apr_size_t)finfo_after.size,
&(new_contents->len));
new_contents->data[new_contents->len] = 0;
Index: ./subversion/libsvn_delta/svndiff.c
===================================================================
--- ./subversion/libsvn_delta/svndiff.c
+++ ./subversion/libsvn_delta/svndiff.c Wed Apr 24 02:36:38 2002
@@ -80,7 +80,7 @@
while (--n >= 0)
{
cont = ((n > 0) ? 0x1 : 0x0) << 7;
- *p++ = ((val >> (n * 7)) & 0x7f) | cont;
+ *p++ = (char)(((val >> (n * 7)) & 0x7f) | cont);
}
return p;
Index: ./subversion/libsvn_delta/text_delta.c
===================================================================
--- ./subversion/libsvn_delta/text_delta.c
+++ ./subversion/libsvn_delta/text_delta.c Wed Apr 24 02:43:02 2002
@@ -421,7 +421,7 @@
* overlap to the beginning of the new buffer. */
if (ab->sbuf_offset + ab->sbuf_len > window->sview_offset)
{
- apr_size_t start = window->sview_offset - ab->sbuf_offset;
+ apr_size_t start = (apr_size_t)(window->sview_offset - ab->sbuf_offset);
memmove (ab->sbuf, old_sbuf + start, ab->sbuf_len - start);
ab->sbuf_len -= start;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Apr 23, 2002 at 03:12:37PM +0200, Sander Striker wrote:
> Hi,
>
> Got lots of warnings on the windows build. This takes care
> of them. Not sure we want to typecast ourselves into oblivion
> though.
>
> This patch deliberately has no log msg (each cast needs review).
>
> Thoughts?
Some of these relate to the issue I filed about adding 'svn_filesize_t'.
Subversion, as an entire system, should be defined to work with 64-bit-sized
files. At least from the API sense. Depending upon the local platform, or
the backend of the FS, or Apache limitations, we might have restrictions
here and there which will drop it. But the *definition* should be 64-bit
throughout.
Note that we only need an unsigned size. If we need canonical values, then
we can simply use 2^64-1, 2^64-2, etc.
>...
> +++ ./subversion/libsvn_fs/strings-table.c Tue Apr 23 13:18:13 2002
> @@ -218,7 +218,7 @@
> svn_fs__clear_dbt (&result);
> result.data = buf;
> result.ulen = *len;
> - result.doff = offset;
> + result.doff = (u_int32_t)offset;
In this case, the 'offset' parameter should be an svn_filesize_t. Thus, this
cast would still be required because we're mapping from the logical space
into an underlying restriction.
>...
> +++ ./subversion/libsvn_fs/reps-strings.c Tue Apr 23 13:18:13 2002
> @@ -445,12 +445,13 @@
> algorithm, and is what has to go when we have a
> true delta combiner. */
> SVN_ERR (rep_read_range (wb->fs, wb->base_rep, sbuf,
> - window->sview_offset, &slen,
> + (apr_size_t)window->sview_offset,
> + &slen,
> wb->trail));
.sview_offset and the parameter to rep_read_range() should be an
svn_filesize_t.
> src_read = 1;
> }
> - memcpy (tbuf + len_read, sbuf + op->offset, op->length);
> - len_read += op->length;
> + memcpy (tbuf + len_read, sbuf + op->offset, (apr_size_t)op->length);
> + len_read += (apr_size_t)op->length;
svn_txdelta_op_t.offset and .length should be an apr_size_t.
>...
> @@ -459,7 +460,7 @@
> /* This could be done in bigger blocks, at the expense
> of some more complexity. */
> int t;
> - for (t = op->offset; t < op->offset + op->length; t++)
> + for (t = (int)op->offset; t < op->offset + op->length; t++)
> tbuf[len_read++] = tbuf[t];
t should be declared as an apr_size_t.
>...
> @@ -468,8 +469,8 @@
> {
> memcpy (tbuf + len_read,
> window->new_data->data + op->offset,
> - op->length);
> - len_read += op->length;
> + (apr_size_t)op->length);
> + len_read += (apr_size_t)op->length;
op->length would be an apr_size_t.
>...
> +++ ./subversion/libsvn_wc/props.c Tue Apr 23 13:10:06 2002
> @@ -659,19 +659,19 @@
> that each .svn subdir remains separable when executing run_log(). */
> str = strstr (base_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
> len = base_prop_tmp_path->data + base_prop_tmp_path->len - str;
> - tmp_prop_base = svn_stringbuf_ncreate (str, len, pool);
> + tmp_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
(->data - str) is a ptroff_t. I'm not sure how that maps into a size_t. I
believe the proper sequence here is to declare 'len' as an apr_size_t, cast
the pointer substruction, then add ->len.
That is: one cast still remains, to convert a ptroff_t into an apr_size_t.
> str = strstr (base_propfile_path->data, SVN_WC_ADM_DIR_NAME);
> len = base_propfile_path->data + base_propfile_path->len - str;
> - real_prop_base = svn_stringbuf_ncreate (str, len, pool);
> + real_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> str = strstr (local_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
> len = local_prop_tmp_path->data + local_prop_tmp_path->len - str;
> - tmp_props = svn_stringbuf_ncreate (str, len, pool);
> + tmp_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> str = strstr (local_propfile_path->data, SVN_WC_ADM_DIR_NAME);
> len = local_propfile_path->data + local_propfile_path->len - str;
> - real_props = svn_stringbuf_ncreate (str, len, pool);
> + real_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
Same all the way through.
>...
> +++ ./subversion/libsvn_wc/log.c Tue Apr 23 13:24:19 2002
> @@ -1095,7 +1095,7 @@
> apr_hash_this (hi, &key, &keylen, &val);
> entry = val;
>
> - if ((keylen == strlen (SVN_WC_ENTRY_THIS_DIR))
> + if ((keylen == (apr_ssize_t)strlen (SVN_WC_ENTRY_THIS_DIR))
> && (strcmp ((char *) key, SVN_WC_ENTRY_THIS_DIR) == 0))
> is_this_dir = TRUE;
keylen must remain an apr_ssize_t, so this seems fine.
However, the test is rather bogus. SVN_WC_ENTRY_THIS_DIR is scanned twice:
once to get the length, and then once to do a comparison. The whole mess can
be simplified to a simple strcmp(), or the Real Man method:
#define KLEN (sizeof(SVN_WC_ENTRY_THIS_DIR)-1)
is_this_dir = keylen == KLEN
&& memcmp(key, SVN_WC_ENTRY_THIS_DIR, KLEN) == 0;
#undef KLEN
(and remove the =FALSE from the declaration for is_this_dir)
>...
> +++ ./subversion/clients/cmdline/util.c Tue Apr 23 13:24:19 2002
> @@ -463,10 +463,10 @@
> else
> {
> /* Read the entire file into memory. */
> - svn_stringbuf_ensure (new_contents, finfo_after.size + 1);
> + svn_stringbuf_ensure (new_contents, (apr_size_t)(finfo_after.size + 1));
> apr_err = apr_file_read_full (tmp_file,
> new_contents->data,
> - finfo_after.size,
> + (apr_size_t)finfo_after.size,
This is fine. We're reading a file (sized as apr_off_t) into memory (sized
as apr_size_t). Gotta keep the cast.
Strictly, you'd want to check the size of the file, to ensure it isn't
larger than memory capacity. Screw that, however :-)
>...
> +++ ./subversion/libsvn_delta/svndiff.c Tue Apr 23 13:24:19 2002
> @@ -80,7 +80,7 @@
> while (--n >= 0)
> {
> cont = ((n > 0) ? 0x1 : 0x0) << 7;
> - *p++ = ((val >> (n * 7)) & 0x7f) | cont;
> + *p++ = (char)(((val >> (n * 7)) & 0x7f) | cont);
Seems fine.
>...
> @@ -341,10 +341,10 @@
> case svn_txdelta_new:
> if (op.length > new_len - npos)
> return -1;
> - npos += op.length;
> + npos += (apr_size_t)op.length;
op.length should be an apr_size_t, as mentioned before.
>...
> @@ -415,22 +415,22 @@
> p = decode_int (&val, p, end);
> if (p == NULL)
> return SVN_NO_ERROR;
> - sview_len = val;
> + sview_len = (apr_size_t)val;
decode_int() should probably be returning an apr_size_t. It isn't possible
to have a window larger than memory-size. This also maps to what will get
marshalled (values from the delta windows which are mostly apr_size_t
values).
>...
> +++ ./subversion/libsvn_delta/text_delta.c Tue Apr 23 13:18:46 2002
> @@ -155,7 +155,7 @@
> op->action_code = opcode;
> op->offset = bob->new_data->len;
> op->length = length;
> - svn_stringbuf_appendbytes (bob->new_data, new_data, length);
> + svn_stringbuf_appendbytes (bob->new_data, new_data, (apr_size_t)length);
svn_txdelta__insert_op() should be declared with svn_filesize_t and
apr_size_t for its offset/length params.
>...
> @@ -421,7 +421,7 @@
> * overlap to the beginning of the new buffer. */
> if (ab->sbuf_offset + ab->sbuf_len > window->sview_offset)
> {
> - apr_size_t start = window->sview_offset - ab->sbuf_offset;
> + apr_size_t start = (apr_size_t)(window->sview_offset - ab->sbuf_offset);
> memmove (ab->sbuf, old_sbuf + start, ab->sbuf_len - start);
> ab->sbuf_len -= start;
> }
This cast is probably needed. You're taking two svn_filesize_t values,
subtracting them, and generating an offset into the window.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Apr 23, 2002 at 06:05:40PM +0200, Sander Striker wrote:
> > From: Karl Fogel [mailto:kfogel@newton.ch.collab.net]
> > Sent: 23 April 2002 17:55
>...
> > Gack. I wonder: is there a flag we can pass to VC to suppress this
> > kind of warning, instead? (Or would that have other undesirable side
> > effects?)
>
> You can surpress the warnings, yes. But, this would mean that we
> ignore usefull warnings.
I'm with Sander here: they should not be ignored. VC is great, in that it
actually helps us sort out these problems. It would be great if GCC had a
similar strictness level so that more of us could participate in finding and
fixing these kinds of problems.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] VC warnings
Posted by Sander Striker <st...@apache.org>.
> From: Karl Fogel [mailto:kfogel@newton.ch.collab.net]
> Sent: 23 April 2002 17:55
> "Sander Striker" <st...@apache.org> writes:
> > Got lots of warnings on the windows build. This takes care
> > of them. Not sure we want to typecast ourselves into oblivion
> > though.
> >
> > This patch deliberately has no log msg (each cast needs review).
> >
> > Thoughts?
>
> Gack. I wonder: is there a flag we can pass to VC to suppress this
> kind of warning, instead? (Or would that have other undesirable side
> effects?)
You can surpress the warnings, yes. But, this would mean that we
ignore usefull warnings.
> This patch may take care of the situation now, but the same problem
> will be constantly arising in future code. I'd hate to be in a
> situation where every code change committed from non-Win32 needs to be
> touched up, after the fact, to placate VC. What a pain that would
> be...
Well, actually, VC is right. Our *nix compilers just aren't as strict
(or the types do actually match on some platforms).
The types _do_ mismatch, and VC complains. This just means we should
be more carefull about our typing.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Sander Striker" <st...@apache.org> writes:
> Got lots of warnings on the windows build. This takes care
> of them. Not sure we want to typecast ourselves into oblivion
> though.
>
> This patch deliberately has no log msg (each cast needs review).
>
> Thoughts?
Gack. I wonder: is there a flag we can pass to VC to suppress this
kind of warning, instead? (Or would that have other undesirable side
effects?)
This patch may take care of the situation now, but the same problem
will be constantly arising in future code. I'd hate to be in a
situation where every code change committed from non-Win32 needs to be
touched up, after the fact, to placate VC. What a pain that would
be...
-K
> Index: ./subversion/libsvn_fs/strings-table.c
> ===================================================================
> --- ./subversion/libsvn_fs/strings-table.c
> +++ ./subversion/libsvn_fs/strings-table.c Tue Apr 23 13:18:13 2002
> @@ -218,7 +218,7 @@
> svn_fs__clear_dbt (&result);
> result.data = buf;
> result.ulen = *len;
> - result.doff = offset;
> + result.doff = (u_int32_t)offset;
> result.dlen = *len;
> result.flags |= (DB_DBT_USERMEM | DB_DBT_PARTIAL);
> db_err = cursor->c_get (cursor, &query, &result, DB_CURRENT);
> Index: ./subversion/libsvn_fs/reps-strings.c
> ===================================================================
> --- ./subversion/libsvn_fs/reps-strings.c
> +++ ./subversion/libsvn_fs/reps-strings.c Tue Apr 23 13:18:13 2002
> @@ -445,12 +445,13 @@
> algorithm, and is what has to go when we have a
> true delta combiner. */
> SVN_ERR (rep_read_range (wb->fs, wb->base_rep, sbuf,
> - window->sview_offset, &slen,
> + (apr_size_t)window->sview_offset,
> + &slen,
> wb->trail));
> src_read = 1;
> }
> - memcpy (tbuf + len_read, sbuf + op->offset, op->length);
> - len_read += op->length;
> + memcpy (tbuf + len_read, sbuf + op->offset, (apr_size_t)op->length);
> + len_read += (apr_size_t)op->length;
> }
> break;
>
> @@ -459,7 +460,7 @@
> /* This could be done in bigger blocks, at the expense
> of some more complexity. */
> int t;
> - for (t = op->offset; t < op->offset + op->length; t++)
> + for (t = (int)op->offset; t < op->offset + op->length; t++)
> tbuf[len_read++] = tbuf[t];
> }
> break;
> @@ -468,8 +469,8 @@
> {
> memcpy (tbuf + len_read,
> window->new_data->data + op->offset,
> - op->length);
> - len_read += op->length;
> + (apr_size_t)op->length);
> + len_read += (apr_size_t)op->length;
> }
> break;
>
> Index: ./subversion/libsvn_wc/props.c
> ===================================================================
> --- ./subversion/libsvn_wc/props.c
> +++ ./subversion/libsvn_wc/props.c Tue Apr 23 13:10:06 2002
> @@ -659,19 +659,19 @@
> that each .svn subdir remains separable when executing run_log(). */
> str = strstr (base_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
> len = base_prop_tmp_path->data + base_prop_tmp_path->len - str;
> - tmp_prop_base = svn_stringbuf_ncreate (str, len, pool);
> + tmp_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> str = strstr (base_propfile_path->data, SVN_WC_ADM_DIR_NAME);
> len = base_propfile_path->data + base_propfile_path->len - str;
> - real_prop_base = svn_stringbuf_ncreate (str, len, pool);
> + real_prop_base = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> str = strstr (local_prop_tmp_path->data, SVN_WC_ADM_DIR_NAME);
> len = local_prop_tmp_path->data + local_prop_tmp_path->len - str;
> - tmp_props = svn_stringbuf_ncreate (str, len, pool);
> + tmp_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> str = strstr (local_propfile_path->data, SVN_WC_ADM_DIR_NAME);
> len = local_propfile_path->data + local_propfile_path->len - str;
> - real_props = svn_stringbuf_ncreate (str, len, pool);
> + real_props = svn_stringbuf_ncreate (str, (apr_size_t)len, pool);
>
> /* Write log entry to move pristine tmp copy to real pristine area. */
> svn_xml_make_open_tag (entry_accum,
> Index: ./subversion/libsvn_wc/log.c
> ===================================================================
> --- ./subversion/libsvn_wc/log.c
> +++ ./subversion/libsvn_wc/log.c Tue Apr 23 13:24:19 2002
> @@ -1095,7 +1095,7 @@
> apr_hash_this (hi, &key, &keylen, &val);
> entry = val;
>
> - if ((keylen == strlen (SVN_WC_ENTRY_THIS_DIR))
> + if ((keylen == (apr_ssize_t)strlen (SVN_WC_ENTRY_THIS_DIR))
> && (strcmp ((char *) key, SVN_WC_ENTRY_THIS_DIR) == 0))
> is_this_dir = TRUE;
>
> Index: ./subversion/clients/cmdline/util.c
> ===================================================================
> --- ./subversion/clients/cmdline/util.c
> +++ ./subversion/clients/cmdline/util.c Tue Apr 23 13:24:19 2002
> @@ -463,10 +463,10 @@
> else
> {
> /* Read the entire file into memory. */
> - svn_stringbuf_ensure (new_contents, finfo_after.size + 1);
> + svn_stringbuf_ensure (new_contents, (apr_size_t)(finfo_after.size + 1));
> apr_err = apr_file_read_full (tmp_file,
> new_contents->data,
> - finfo_after.size,
> + (apr_size_t)finfo_after.size,
> &(new_contents->len));
> new_contents->data[new_contents->len] = 0;
>
> Index: ./subversion/libsvn_delta/svndiff.c
> ===================================================================
> --- ./subversion/libsvn_delta/svndiff.c
> +++ ./subversion/libsvn_delta/svndiff.c Tue Apr 23 13:24:19 2002
> @@ -80,7 +80,7 @@
> while (--n >= 0)
> {
> cont = ((n > 0) ? 0x1 : 0x0) << 7;
> - *p++ = ((val >> (n * 7)) & 0x7f) | cont;
> + *p++ = (char)(((val >> (n * 7)) & 0x7f) | cont);
> }
>
> return p;
> @@ -341,10 +341,10 @@
> case svn_txdelta_new:
> if (op.length > new_len - npos)
> return -1;
> - npos += op.length;
> + npos += (apr_size_t)op.length;
> break;
> }
> - tpos += op.length;
> + tpos += (apr_size_t)op.length;
> if (tpos < 0)
> return -1;
> n++;
> @@ -415,22 +415,22 @@
> p = decode_int (&val, p, end);
> if (p == NULL)
> return SVN_NO_ERROR;
> - sview_len = val;
> + sview_len = (apr_size_t)val;
>
> p = decode_int (&val, p, end);
> if (p == NULL)
> return SVN_NO_ERROR;
> - tview_len = val;
> + tview_len = (apr_size_t)val;
>
> p = decode_int (&val, p, end);
> if (p == NULL)
> return SVN_NO_ERROR;
> - inslen = val;
> + inslen = (apr_size_t)val;
>
> p = decode_int (&val, p, end);
> if (p == NULL)
> return SVN_NO_ERROR;
> - newlen = val;
> + newlen = (apr_size_t)val;
>
> /* Check for integer overflow (don't want to let the input trick
> us into invalid pointer games using negative numbers). */
> @@ -477,7 +477,7 @@
> if (op->action_code == svn_txdelta_new)
> {
> op->offset = npos;
> - npos += op->length;
> + npos += (apr_size_t)op->length;
> }
> }
> window.num_ops = ninst;
> Index: ./subversion/libsvn_delta/text_delta.c
> ===================================================================
> --- ./subversion/libsvn_delta/text_delta.c
> +++ ./subversion/libsvn_delta/text_delta.c Tue Apr 23 13:18:46 2002
> @@ -155,7 +155,7 @@
> op->action_code = opcode;
> op->offset = bob->new_data->len;
> op->length = length;
> - svn_stringbuf_appendbytes (bob->new_data, new_data, length);
> + svn_stringbuf_appendbytes (bob->new_data, new_data, (apr_size_t)length);
> break;
> default:
> assert (!"unknown delta op.");
> @@ -351,8 +351,8 @@
> case svn_txdelta_source:
> /* Copy from source area. */
> assert (op->offset + op->length <= window->sview_len);
> - memcpy (tbuf + tpos, sbuf + op->offset, op->length);
> - tpos += op->length;
> + memcpy (tbuf + tpos, sbuf + op->offset, (apr_size_t)op->length);
> + tpos += (apr_size_t)op->length;
> break;
>
> case svn_txdelta_target:
> @@ -361,7 +361,7 @@
> and target copies are allowed to overlap to generate
> repeated data. */
> assert (op->offset < tpos);
> - for (i = op->offset; i < op->offset + op->length; i++)
> + for (i = (apr_size_t)op->offset; i < op->offset + op->length; i++)
> tbuf[tpos++] = tbuf[i];
> break;
>
> @@ -370,8 +370,8 @@
> assert (op->offset + op->length <= window->new_data->len);
> memcpy (tbuf + tpos,
> window->new_data->data + op->offset,
> - op->length);
> - tpos += op->length;
> + (apr_size_t)op->length);
> + tpos += (apr_size_t)op->length;
> break;
>
> default:
> @@ -421,7 +421,7 @@
> * overlap to the beginning of the new buffer. */
> if (ab->sbuf_offset + ab->sbuf_len > window->sview_offset)
> {
> - apr_size_t start = window->sview_offset - ab->sbuf_offset;
> + apr_size_t start = (apr_size_t)(window->sview_offset - ab->sbuf_offset);
> memmove (ab->sbuf, old_sbuf + start, ab->sbuf_len - start);
> ab->sbuf_len -= start;
> }
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
"Sander Striker" <st...@apache.org> writes:
> Ofcourse Bill Tutt was the first to bonk me on the head for this.
> We need to fix the typing rather than doing casts. There were
> a few legit casts IMHO, but I agree with Bill.
Oh, were they a matter of using the wrong types anyway? In that case
ignore my previous mail, heh; Bill's fix is most definitely the right
one.
> Is this in the issue db somewhere already? I couldn't find
> one, but didn't search very hard.
I don't think it is -- file & own, please! :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Apr 23, 2002 at 10:29:01PM +0200, Branko Äibej wrote:
> Sander Striker wrote:
>...
> >Is this in the issue db somewhere already? I couldn't find
> >one, but didn't search very hard.
>
> I think there's something there about using a 64-bit type for file
> offsets everywhere (instead of apr_off_t, which isn't consistent across
> platforms), which resulted from discussion about these warnings.
Issue 639: http://subversion.tigris.org/issues/show_bug.cgi?id=639
[ hunh. in my earlier note, I indicated that svn_filesize_t would be
unsigned, but in the issue, I said it would be signed... hrm... ]
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] VC warnings
Posted by Branko Čibej <br...@xbc.nu>.
Sander Striker wrote:
>>From: Sander Striker [mailto:striker@apache.org]
>>Sent: 23 April 2002 15:13
>>
>
>>Hi,
>>
>>Got lots of warnings on the windows build. This takes care
>>of them. Not sure we want to typecast ourselves into oblivion
>>though.
>>
>>This patch deliberately has no log msg (each cast needs review).
>>
>>Thoughts?
>>
>
>Ofcourse Bill Tutt was the first to bonk me on the head for this.
>We need to fix the typing rather than doing casts. There were
>a few legit casts IMHO, but I agree with Bill.
>
Yep, that's exactly why I haven't done it, and tolerate the warnings
instead.
>Is this in the issue db somewhere already? I couldn't find
>one, but didn't search very hard.
>
I think there's something there about using a 64-bit type for file
offsets everywhere (instead of apr_off_t, which isn't consistent across
platforms), which resulted from discussion about these warnings.
--
Brane Čibej <br...@xbc.nu> http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: [PATCH] VC warnings
Posted by Sander Striker <st...@apache.org>.
> From: Sander Striker [mailto:striker@apache.org]
> Sent: 23 April 2002 15:13
> Hi,
>
> Got lots of warnings on the windows build. This takes care
> of them. Not sure we want to typecast ourselves into oblivion
> though.
>
> This patch deliberately has no log msg (each cast needs review).
>
> Thoughts?
Ofcourse Bill Tutt was the first to bonk me on the head for this.
We need to fix the typing rather than doing casts. There were
a few legit casts IMHO, but I agree with Bill.
Is this in the issue db somewhere already? I couldn't find
one, but didn't search very hard.
Sander
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org