Re: [PATCH] add warnings enum-to-int and int-to-enum

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On Wed, 2 Sep 2009, Josh Triplett wrote:

> On Wed, Sep 02, 2009 at 02:43:52PM -0400, Daniel Barkalow wrote:
> > On Wed, 2 Sep 2009, Kamil Dudka wrote:
> > 
> > > On Wednesday 02 of September 2009 19:56:47 Daniel Barkalow wrote:
> > > > It feels to me like the explicit numeric values are what make these
> > > > constants sensible to use directly as ints, and that it's only sensible to
> > > > use a non-constant value of an enum type as an int (without an explicit
> > > > cast) if all of the enum values have explicit numeric values.
> > > >
> > > > I think:
> > > >
> > > >   enum {
> > > >     my_register_zero
> > > >     ...
> > > >     my_register_twdr
> > > >     my_register_twcr
> > > >     ...
> > > >   };
> > > >
> > > >   void () {
> > > >     write_register(my_register_twdr, SETUP_TWDR);
> > > >   }
> > > >
> > > > is asking for trouble in a way that this warning is about.
> > > 
> > > Both examples are too abstract for me -- missing declaration
> > > of write_register(), etc. Please attach a minimal example as a file which I 
> > > can compile and test. I'll check if the "trouble" is covered by the warnings 
> > > or not, and perhaps implement what's missing. Thanks in advance!
> > 
> > enum {
> >   foo,
> >   bar
> > };
> > 
> > enum {
> >   baz = 1,
> >   qux = 2
> > };
> > 
> > void test(void) {
> >   int i = bar; // warn on this
> >   int j = qux; // okay
> > }
> > 
> > (Leaving aside the issue of whether the enum is anonymous)
> > 
> > In the "bar" case, an additional value added somewhere in the list 
> > (particularly if the list were long) might change "i" in a way that 
> > wouldn't necessarily be obvious to users of "i". In the "qux" case, "j" 
> > would only change if the "qux = 2" line were changed.
> 
> I disagree with this.  In both cases, you've declared an anonymous enum,
> and then used its values as constants.  In the former case, you might
> just not care about the values except to compare against each other;
> granted, you ought to use a named enum rather than an int in that case,
> but nevertheless much code exists that uses int instead of enum types.

I think anonymous vs named enums are one thing that should affect whether 
you get a warning, and explicit values vs implicit values are another 
factor, but I don't have an opinion on which way they should combine. 
Probably either using an explicit value or an anonymous enum should be 
okay by default, and the test above should use named enums.

	-Daniel
*This .sig left intentionally blank*
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Newbies FAQ]     [Kernel List]     [Site Home]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Photo]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [DDR & Rambus]     [Trinity Fuzzer Tool]

Powered by Linux