Re: [PATCH 1/4] NTFS: Fix sparse warnings that have crept in over time.

[Linus Torvalds <torvalds@osdl.org>]

On Mon, 26 Sep 2005, Anton Altaparmakov wrote:
>
> NTFS: Fix sparse warnings that have crept in over time.

I think this is wrong.

What was the warning that caused this (and the two other things that look 
the same)?

>  #define MK_MREF(m, s)	((MFT_REF)(((MFT_REF)(s) << 48) |		\
> -					((MFT_REF)(m) & MFT_REF_MASK_CPU)))
> +					((MFT_REF)(m) & (u64)MFT_REF_MASK_CPU)))

Also, side note: how you defined "MFT_REF_MASK_CPU" is pretty debatable in 
the first place:

	typedef enum {
	        MFT_REF_MASK_CPU        = 0x0000ffffffffffffULL,
	        MFT_REF_MASK_LE         = const_cpu_to_le64(0x0000ffffffffffffULL),
	} MFT_REF_CONSTS;

and this just _happens_ to work with gcc, but it's not real C.

The issue? "enum" is really an integer type. As in "int". Trying to put a 
larger value than one that fits in "int" is not guaranteed to work.

There's another issue, namely that the type of the snum is not only of 
undefined size (is it the same size as an "int"? Is it an "unsigned long 
long"?) but the "endianness" of it is also now totally undefined. You have 
two different endiannesses inside the _same_ enum. What is the type of the 
enum?

You _really_ probably should use just

	#define MFT_REF_MASK_CPU 0x0000ffffffffffffULL
	#define MFT_REF_MASK_LE const_cpu_to_le64(MFT_REF_MASK_CPU)

instead. That way the type of that thing is well-defined.

Depending on what warning you're trying to shut up, that may well fix it 
too. Because now "MFT_REF_MASK_CPU" is clearly a regular constant, while 
MFT_REF_MASK_LE is clearly a little-endian constant. Before, they were of 
the same enum type, which made it very unclear what the hell they were.

		Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/