On Mon, May 29, 2006 at 12:08:25PM +1000, Neil Brown wrote:
On Sunday May 28, bluca@xxxxxxxxxx wrote:
Thanks for the patches. They are greatly appreciated.
You're welcome
- mdadm-2.3.1-kernel-byteswap-include-fix.patch
reverts a change introduced with mdadm 2.3.1 for redhat compatibility
asm/byteorder.h is an architecture dependent file and does more
stuff than a call to the linux/byteorder/XXX_endian.h
the fact that not calling asm/byteorder.h does not define
__BYTEORDER_HAS_U64__ is just an example of issues that might arise.
if redhat is broken it should be worked around differently than breaking
mdadm.
I don't understand the problem here. What exactly breaks with the
code currently in 2.5? mdadm doesn't need __BYTEORDER_HAS_U64__, so
why does not having id defined break anything?
The coomment from the patch says:
> not including asm/byteorder.h will not define __BYTEORDER_HAS_U64__
> causing __fswab64 to be undefined and failure compiling mdadm on
> big_endian architectures like PPC
But again, mdadm doesn't use __fswab64 ....
More details please.
you use __cpu_to_le64 (ie in super0.c line 987)
bms->sync_size = __cpu_to_le64(size);
which in byteorder/big_endian.h is defined as
#define __cpu_to_le64(x) ((__force __le64)__swab64((x)))
but __swab64 is defined in byteorder/swab.h (included by
byteorder/big_endian.h) as
#if defined(__GNUC__) && (__GNUC__ >= 2) && defined(__OPTIMIZE__)
# define __swab64(x) \
(__builtin_constant_p((__u64)(x)) ? \
___swab64((x)) : \
__fswab64((x)))
#else
# define __swab64(x) __fswab64(x)
#endif /* OPTIMIZE */
and __fswab64 is defined further into byteorder/swab.h as
#ifdef __BYTEORDER_HAS_U64__
static __inline__ __attribute_const__ __u64 __fswab64(__u64 x)
.....
#endif /* __BYTEORDER_HAS_U64__ */
so building mdadm on a ppc (or i suppose a sparc) will break
now, if you look at /usr/src/linux/asm-*/byteorder.h you will notice
they are very different files, this makes me believe it is not a good
idea bypassing asm/byteorder.h
And no, just defining __BYTEORDER_HAS_U64__ will break on 32bit
big-endian cpus (and if i do not misread it might just compile and give
incorrect results)
- mdadm-2.4-snprintf.patch
this is self commenting, just an error in the snprintf call
I wonder how that snuck in...
There was an odd extra tab in the patch, but no-matter.
I changed it to use 'sizeof(buf)' to be consistent with other uses
of snprint. Thanks.
yes, that would be better.
- mdadm-2.4-strict-aliasing.patch
fix for another srict-aliasing problem, you can typecast a reference to a
void pointer to anything, you cannot typecast a reference to a
struct.
Why can't I typecast a reference to a struct??? It seems very
unfair...
that's strict-aliasing optimization for you, i do agree it _is_ unfair.
However I have no problem with the patch. Applied. Thanks.
I should really change it to use 'list.h' type lists from the linux
kernel.
hopefull redhat would not object :)
- mdadm-2.5-mdassemble.patch
pass CFLAGS to mdassemble build, enabling -Wall -Werror showed some
issues also fixed by the patch.
yep, thanks.
- mdadm-2.5-rand.patch
Posix dictates rand() versus bsd random() function, and dietlibc
deprecated random(), so switch to srand()/rand() and make everybody
happy.
Everybody?
'man 3 rand' tells me:
Do not use this function in applications intended to be
portable when good randomness is needed.
Admittedly mdadm doesn't need to be portable - it only needs to run on
Linux. But this line in the man page bothers me.
I guess
-Drandom=rand -Dsrandom=srand
might work.... no. stdlib.h doesn't like that.
'random' returns 'long int' while rand returns 'int'.
Interestingly 'random_r' returns 'int' as does 'rand_r'.
#ifdef __dietlibc__
#include <strings.h>
/* dietlibc has deprecated random and srandom!! */
#define random rand
#define srandom srand
#endif
in mdadm.h. Will that do you?
yes, mdassemble will build, so it is ok for me.
- mdadm-2.5-unused.patch
glibc 2.4 is pedantic on ignoring return values from fprintf, fwrite and
write, so now we check the rval and actually do something with it.
in the Grow.c case i only print a warning, since i don't think we can do
anithing in case we fail invalidating those superblocks (is should never
happen, but then...)
Ok, thanks.
You can see these patches at
http://neil.brown.name/cgi-bin/gitweb.cgi?p=mdadm
Thanks.
--
Luca Berra -- bluca@xxxxxxxxxx
Communication Media & Services S.r.l.
/"\
\ / ASCII RIBBON CAMPAIGN
X AGAINST HTML MAIL
/ \
-
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
