[U-Boot] [PATCH] dm: video: fix abuse of enum

Marcel Ziswiler marcel.ziswiler at toradex.com
Thu Jul 6 12:22:52 UTC 2017


Dear Lothar

On Thu, 2017-07-06 at 09:50 +0200, Lothar Waßmann wrote:
> Hi,
> 
> On Wed, 5 Jul 2017 22:49:28 -0600 Simon Glass wrote:
> > Hi Lothar,
> > 
> > On 23 June 2017 at 00:30, Lothar Waßmann <LW at karo-electronics.de>
> > wrote:
> > > Hi,
> > > 
> > > On Wed, 21 Jun 2017 09:59:05 +0200 Lothar Waßmann wrote:
> > > > Hi,
> > > > 
> > > > On Tue, 20 Jun 2017 12:26:29 -0600 Simon Glass wrote:
> > > > > Hi Lothar,
> > > > > 
> > > > > On 20 June 2017 at 04:25, Lothar Waßmann <LW at karo-electronics
> > > > > .de> wrote:
> > > > > > LCD_MAX_WIDTH, LCD_MAX_HEIGHT and LCD_MAX_LSBPP are not
> > > > > > alternative
> > > > > > values for one specific variable, but unrelated entities
> > > > > > with distinct
> > > > > > purposes. There is no use defining them as values of an
> > > > > > 'enum'.
> > > > > 
> > > > > Can you explain why #define is better? I prefer enum since
> > > > > they are a
> > > > > compiler construct instead of preprocessor (thus no need for
> > > > > brackets,
> > > > > no strange conversion things) and the debugger knows about
> > > > > them.
> > > > > 
> > > > 
> > > > An enum defines alternative values for one specific entity
> > > > (e.g.
> > > > clauses for a switch construct), but not a collection of
> > > > arbitrary data
> > > > items.
> > > > 
> > > > > > The 'enum' construct would fail miserably for an LCD
> > > > > > controller that
> > > > > > has a square max. frame size (e.g. 4096x4096).
> > > > > 
> > > > > What does this mean? I don't understand sorry.
> > > > > 
> > > > 
> > > > Try your enum with MAX_LCD_WITDH == MAC_LCD_HEIGHT.
> > 
> > Can you please be explicit as to what the problem is? Sorry but I
> > don't understand what you are driving at. Do you have a test
> > program
> > which shows the problem?
> > 
> 
> You cannot have two different enum items with the same value!
> Thus:
> enum {
> 	MAX_LCD_WIDTH = 4096,
> 	MAX_LCD_HEIGHT = 4096,
> };
> won't compile.

Says who?

At least my gcc compilers even compile the following just fine:

enum {
	MAX_LCD_WIDTH = 4096,
	MAX_LCD_HEIGHT = MAX_LCD_WIDTH,
};

I really don't think you have any valid point here. Enum items are
really just numbers and there is no limitation as such whether two
items may share the same number or not. It's really absolutely OK. Of
course as such one can not really guess the exact enum item/name from
just the number which may sound suboptimal but that is completely legal
in C. Some other high level languages do not allow that as far as I
remember (e.g. Java).

> The purpose of an enum is to provide a collection of possible values
> that can be taken by a single variable. E.g. enumerate the states of
> a
> state machine, video modes, CPU types...
> It's not meant to group together otherwise unsolicited values.
> 
> 
> Lothar Waßmann

Cheers

Marcel


More information about the U-Boot mailing list