Thread (1 message) 1 message, 1 author, 2009-06-08

Re: ANNOUNCE: mdadm 3.0 - A tool for managing Soft RAID under Linux

From: Neil Brown <hidden>
Date: 2009-06-08 23:36:30

On Saturday June 6, davidsen@tmr.com wrote:
Neil Brown wrote:
quoted
On Wednesday June 3, babydr@baby-dragons.com wrote:
  
quoted
 	Hello Neil ,  I am getting a interesting Error during compiling 3.0 .
 	Is there a particular version of kernel that 3.0 is supposed to be 
compiled with ?
    
This has nothing to do with kernel version.  You must be using a
different compiler version - it is picking up an error that might
didn't.

The fix is below.
Thanks,
NeilBrown


  
quoted
 		Tia ,  JimL

gcc -Wall -Werror -Wstrict-prototypes -ggdb -DSendmail=\""/usr/sbin/sendmail 
-t"\" -DCONFFILE=\"/etc/mdadm.conf\" -DCONFFILE2=\"/etc/mdadm/mdadm.conf\"   -c 
-o super-intel.o super-intel.c
cc1: warnings being treated as errors
super-intel.c: In function 'mark_failure':
super-intel.c:3632: warning: comparison is always false due to limited range of 
data type
make: *** [super-intel.o] Error 1

    
commit 4291d691b66f65695b5b4be22b80fd00da73b544
Author: NeilBrown [off-list ref]
Date:   Thu Jun 4 12:29:21 2009 +1000

    super-intel: fix test on failed_disk_num.
    
    We sometimes set failed_disk_num to ~0.
    However we cannot test for equality with that as  failed_disk_num
    is 8bit and ~0 is probably 32bit with lots of 1's.
    So test if ~failed_disk_num is 0 instead.
    
    Reported-By: "Mr. James W. Laferriere" [off-list ref]
    Signed-off-by: NeilBrown [off-list ref]
diff --git a/super-intel.c b/super-intel.c
index 73fe5fa..7e2a086 100644
--- a/super-intel.c
+++ b/super-intel.c
@@ -3629,7 +3629,7 @@ static int mark_failure(struct imsm_dev *dev, struct imsm_disk *disk, int idx)
 
 	disk->status |= FAILED_DISK;
 	set_imsm_ord_tbl_ent(map, slot, idx | IMSM_ORD_REBUILD);
-	if (map->failed_disk_num == ~0)
+	if (~map->failed_disk_num == 0)
 		map->failed_disk_num = slot;
 	return 1;
 }
  
I still don't think this is really portable, the zero should be cast 
using typeof.
????

zero is zero is zero.
A cast will either add zero bits or remove zero bits, the net result
is always the same.

"-1" is different.  Casting it could add zeros or ones depending on
whether it seems to be signed at the time.  That was the original
problem.  failed_disk_num is unsigned 8 bits.  So when we assign ~0
to it, it becomes 0b11111111.
But when it is implicitly cast to an int for the comparison, it
becomes
   0b00000000000000000000000011111111
which is very different from ~0 which is
   0b11111111111111111111111111111111

So I stand by the new code.

Thanks,
NeilBrown
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help