[PATCH] udf: uint32_t can't be less than zero

[PATCH] udf: uint32_t can't be less than zero

[Jesper Juhl]

Here's a patch that removes a few bits from fs/udf/balloc.c that 
test uint32_t values for being less than zero, which is impossible.

I know not everyone agree with this sort of cleanup, but I figured I'd do 
the patch in any case, then leave it up to the maintainer to apply it or 
drop it.

Please keep me on CC: when replying.



Signed-Off-by: Jesper Juhl <juhl-lkml@dif.dk>


--- linux-2.6.12-rc2-mm3-orig/fs/udf/balloc.c	2005-03-02 08:37:52.000000000 +0100
+++ linux-2.6.12-rc2-mm3/fs/udf/balloc.c	2005-04-23 20:38:31.000000000 +0200
@@ -158,8 +158,7 @@ static void udf_bitmap_free_blocks(struc
 	unsigned long overflow;
 
 	down(&sbi->s_alloc_sem);
-	if (bloc.logicalBlockNum < 0 ||
-		(bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))
+	if ((bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))
 	{
 		udf_debug("%d < %d || %d + %d > %d\n",
 			bloc.logicalBlockNum, 0, bloc.logicalBlockNum, count,
@@ -232,7 +231,7 @@ static int udf_bitmap_prealloc_blocks(st
 	struct buffer_head *bh;
 
 	down(&sbi->s_alloc_sem);
-	if (first_block < 0 || first_block >= UDF_SB_PARTLEN(sb, partition))
+	if (first_block >= UDF_SB_PARTLEN(sb, partition))
 		goto out;
 
 	if (first_block + block_count > UDF_SB_PARTLEN(sb, partition))
@@ -299,7 +298,7 @@ static int udf_bitmap_new_block(struct s
 	down(&sbi->s_alloc_sem);
 
 repeat:
-	if (goal < 0 || goal >= UDF_SB_PARTLEN(sb, partition))
+	if (goal >= UDF_SB_PARTLEN(sb, partition))
 		goal = 0;
 
 	nr_groups = bitmap->s_nr_groups;
@@ -439,8 +438,7 @@ static void udf_table_free_blocks(struct
 	int i;
 
 	down(&sbi->s_alloc_sem);
-	if (bloc.logicalBlockNum < 0 ||
-		(bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))
+	if ((bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))
 	{
 		udf_debug("%d < %d || %d + %d > %d\n",
 			bloc.logicalBlockNum, 0, bloc.logicalBlockNum, count,
@@ -688,7 +686,7 @@ static int udf_table_prealloc_blocks(str
 	struct buffer_head *bh;
 	int8_t etype = -1;
 
-	if (first_block < 0 || first_block >= UDF_SB_PARTLEN(sb, partition))
+	if (first_block >= UDF_SB_PARTLEN(sb, partition))
 		return 0;
 
 	if (UDF_I_ALLOCTYPE(table) == ICBTAG_FLAG_AD_SHORT)
@@ -768,7 +766,7 @@ static int udf_table_new_block(struct su
 		return newblock;
 
 	down(&sbi->s_alloc_sem);
-	if (goal < 0 || goal >= UDF_SB_PARTLEN(sb, partition))
+	if (goal >= UDF_SB_PARTLEN(sb, partition))
 		goal = 0;
 
 	/* We search for the closest matching block to goal. If we find a exact hit,

Re: [PATCH] udf: uint32_t can't be less than zero

[Denis Vlasenko]

On Saturday 23 April 2005 21:48, Jesper Juhl wrote:
> Here's a patch that removes a few bits from fs/udf/balloc.c that 
> test uint32_t values for being less than zero, which is impossible.
> 
> I know not everyone agree with this sort of cleanup, but I figured I'd do 
> the patch in any case, then leave it up to the maintainer to apply it or 
> drop it.
> 
> Please keep me on CC: when replying.
> -	if (bloc.logicalBlockNum < 0 ||
> -		(bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))
> +	if ((bloc.logicalBlockNum + count) > UDF_SB_PARTLEN(sb, bloc.partitionReferenceNum))

It is not immediately visible here that bloc.logicalBlockNum is unsigned.
One needs to check that by looking at the definition.

Also if later someone changes bloc.logicalBlockNum into signed entity, code
becomes buggy. Not good.

gcc already optimizes out such checks:

# gcc -O2 t.c -S -fomit-frame-pointer
# cat t.c t.s
extern unsigned v;

int f() {
    return v<0 || v>100 || v==50;
}
        .file   "t.c"
        .text
        .p2align 2,,3
.globl f
        .type   f, @function
f:
        movl    v, %edx
        xorl    %eax, %eax
        cmpl    $100, %edx
        ja      .L3
        cmpl    $50, %edx
        je      .L3
        ret
        .p2align 2,,3
.L3:
        movl    $1, %eax
        ret
        .size   f, .-f
        .section        .note.GNU-stack,"",@progbits
        .ident  "GCC: (GNU) 3.4.3"
--
vda