Re: [PATCH 8/10] Re: [2.6-BK-URL] NTFS: 2.1.19 sparse annotation,cleanups and a bugfix

From: Anton Altaparmakov
Date: Sun Sep 26 2004 - 02:53:51 EST


On Sun, 26 Sep 2004, Anton Altaparmakov wrote:

> On Fri, 24 Sep 2004, Linus Torvalds wrote:
> > On Fri, 24 Sep 2004, Anton Altaparmakov wrote:
> > > On Fri, 24 Sep 2004, Linus Torvalds wrote:
> > > >
> > > > Btw, Al is fixing this. We'll make enum's properly typed, rather than just
> > > > plain integers. It's not traditional C behaviour, but it gives you better
> > > > type safety, and Al points out that other C compilers (the Plan 9 one, to
> > > > be specific) have done the same thing for similar reasons.
> >
> > Well, when I said "Al is fixing this", I lied.
> >
> > I just fixed it myself.
>
> Great. (-:
>
> > > This is good news. Once that is done I will be very happy to go back to
> > > using enums as I also agree that they can and in this case do look a
> > > lot nicer...
> >
> > Try the current sparse, I think it should work for you.
> >
> > So if you make an enum where the initializer expression is a little-endian
> > expression, the type of that (single) enumerator will be little-endian.
> >
> > HOWEVER, the type of an enum _variable_ will still be just "int". So
> >
> > enum myenum {
> > one = 1ULL,
> > two = 2,
> > };
> >
> > has the strange behaviour that if you use "one" in an expression, it will
> > have the type "unsigned long long", but if you use a "enum myenum" entry
> > (even if it has the value "1"), it will be an "int":
> >
> > sizeof(one) == 8
> > sizeof(enum myenum) == 4
> >
> > So I would stronly suggest (and I may make sparse warn) against using
> > non-integertyped enum values with any enum that actually has any backing
> > store (ie if you ever use a variable of type "enum myenum", that would
> > result in a warning - you can really just use the values "one" and "two"
> > directly).
>
> Ah, I was using them for backing store as well and I was using the
> __attribute__((packed)) gcc extension to make them the bit-width I wanted
> in combination with a "filler element" at the end of the enum.
>
> So for example to get a 16-bit enum type I was using:
>
> typedef enum {
> RESTART_VOLUME_IS_CLEAN = const_cpu_to_le16(0x0002),
> REST_AREA_SPACE_FILLER = 0xffff /* Just to make flags
> 16-bit. */
> } __attribute__ ((__packed__)) RESTART_AREA_FLAGS;
>
> And then when defining the structure containing these flags I would just
> do:
>
> typedef struct {
> ...
> RESTART_AREA_FLAGS flags;
> ...
> } __attribute__ ((__packed__)) RESTART_AREA;
>
> Also I use the enum type as parameters to functions, for example in the
> above case I might have:
> int blah(RESTART_AREA_FLAGS flags);
>
> So this use doesn't work with the sparse update either. At the moment I
> have changed everything to just a bunch of #defines followed by a:
>
> typedef le16 RESTART_AREA_FLAGS;
>
> So I guess with your sparse update I can now go to a point in between the
> old one and the new one:
>
> enum {
> RESTART_VOLUME_IS_CLEAN = const_cpu_to_le16(0x0002),
> } __attribute__ ((__packed__)) RESTART_AREA_FLAGS;

Silly cut'n'paste error. I meant:

> } __attribute__ ((__packed__));

>
> typedef le16 RESTART_AREA_FLAGS;
>
> So I get the enum rather than bunch of defines and I get my proper types
> as well.
>
> That only looses the ability for the compiler to warn if people use the
> wrong constant when trying to set such a variable or pass a wrong constant
> into a function but that is not nearly as useful a warning as the wrong
> endianness bitwise warnings we have now gained so I am not going to worry
> about losing it.
>
> Best regards,
>
> Anton
>

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/