You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2001/10/01 10:09:04 UTC

Re: svn commit: rev 182 - trunk/subversion/libsvn_fs

On Fri, Sep 28, 2001 at 07:33:05PM -0500, brane@tigris.org wrote:
> Author: brane
> Date: 2001-09-29 00:33 GMT
> New Revision: 182
> 
> Modified:
>    trunk/subversion/libsvn_fs/key-gen.c
>    trunk/subversion/libsvn_fs/skel.c
>    trunk/subversion/libsvn_fs/txn-table.c
>    trunk/subversion/libsvn_fs/validate.c
> Log:
> Fix signed/unsigned comparison warnings.
>...
> --- OLD/trunk/subversion/libsvn_fs/key-gen.c	Fri Sep 28 19:33:05 2001
> +++ NEW/trunk/subversion/libsvn_fs/key-gen.c	Fri Sep 28 19:33:05 2001
> @@ -41,14 +41,14 @@
>  
>       So we do the check for overflow before we multiply value and add
>       in the new digit.  */
> -  int max_prefix = max / 10;
> -  int max_digit = max % 10;
> -  int i;
> +  apr_size_t max_prefix = max / 10;
> +  apr_size_t max_digit = max % 10;
> +  apr_size_t i;

This is bogus. Digits are integers. The problem is with the "max" parameter,
not the internal values. A maximum integer shouldn't be an apr_size_t. Some
other type ought to be used.

apr_size_t is for memory sizes.

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: svn commit: rev 182 - trunk/subversion/libsvn_fs

Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:

>On Fri, Sep 28, 2001 at 07:33:05PM -0500, brane@tigris.org wrote:
>
>>Author: brane
>>Date: 2001-09-29 00:33 GMT
>>New Revision: 182
>>
>>Modified:
>>   trunk/subversion/libsvn_fs/key-gen.c
>>   trunk/subversion/libsvn_fs/skel.c
>>   trunk/subversion/libsvn_fs/txn-table.c
>>   trunk/subversion/libsvn_fs/validate.c
>>Log:
>>Fix signed/unsigned comparison warnings.
>>...
>>--- OLD/trunk/subversion/libsvn_fs/key-gen.c	Fri Sep 28 19:33:05 2001
>>+++ NEW/trunk/subversion/libsvn_fs/key-gen.c	Fri Sep 28 19:33:05 2001
>>@@ -41,14 +41,14 @@
>> 
>>      So we do the check for overflow before we multiply value and add
>>      in the new digit.  */
>>-  int max_prefix = max / 10;
>>-  int max_digit = max % 10;
>>-  int i;
>>+  apr_size_t max_prefix = max / 10;
>>+  apr_size_t max_digit = max % 10;
>>+  apr_size_t i;
>>
>
>This is bogus. Digits are integers. The problem is with the "max" parameter,
>not the internal values. A maximum integer shouldn't be an apr_size_t. Some
>other type ought to be used.
>
First, these aren't maximum or minimum numbers, they're digit counters. 
Second, the other ways to avoid the warnings are a) use casts (blech), 
or b) change the declaration of the skel's length field, which would 
open a whole new can of worms.

I would agree with you in principle; unfortunately, this is C, a totally 
unprincipled language, where the results of comparisons between signed 
and unsigned quantities are undefined. I do /not/ want to add a horde of 
casts to the code (the one I did was two too many ...), nor do I want to 
see these warnings.

>apr_size_t is for memory sizes.
>
Except for "i", these are memory sizes.


-- 
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