lkml.org 
[lkml]   [2005]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/4] NTFS: Fix sparse warnings that have crept in over time.


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/

\
 
 \ /
  Last update: 2009-11-18 23:46    [from the cache]
©2003-2011 Jasper Spaans