linux-kernel - Re: [PATCH 1/2] swap led_brightness from enum to typedef

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210803213855.GA30387@amd>
Date:   Tue, 3 Aug 2021 23:38:56 +0200
From:   Pavel Machek <pavel@....cz>
To:     Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Amy Parker <apark0006@...dent.cerritos.edu>,
        linux-leds <linux-leds@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: [PATCH 1/2] swap led_brightness from enum to typedef

Hi!

> > > The TODO located at drivers/leds/TODO requests:
> > >
> > > * Get rid of led_brightness
> > >
> > > It is really an integer, as maximum is configurable. Get rid of it, or
> > > make it into typedef or something.
> > >
> > > This patch changes the declaration of led_brightness from an enum to a
> > > typedef. In order to hold the currently existing enum values, macro
> > > definitions are provided. Files which use led_brightness are updated to
> > > conform to the new types.
> > >
> > > Signed-off-by: Amy Parker <apark0006@...dent.cerritos.edu>
> >
> > Thanks for your patch!
> >
> > > 207 files changed, 437 insertions(+), 438 deletions(-)
> >
> > This touches a lot of files, so we better get it right.
> >
> > > --- a/include/linux/leds.h
> > > +++ b/include/linux/leds.h
> > > @@ -26,12 +26,11 @@ struct device_node;
> > >  */
> > >
> > > /* This is obsolete/useless. We now support variable maximum brightness. */
> > > -enum led_brightness {
> > > -     LED_OFF         = 0,
> > > -     LED_ON          = 1,
> > > -     LED_HALF        = 127,
> > > -     LED_FULL        = 255,
> > > -};
> > > +typedef u8 led_brightness;
> >
> > In general, typedefs are frowned upon in the kernel, but there can be a
> > good reason to use one.
> > What if the maximum brightness is larger than 255?
> > Using "unsigned int" sounds better to me, but let's wait for Pavel...
> 
> And as Dan just pointed out, "signed int" would be even better, as it
> would allow a function to return an error code.

We can not apply huge patch all at once, and changing from enum to int
directly will result in warnings in some configurations, no?

Agreed that int would be good.

Best regards,
								Pavel

-- 
http://www.livejournal.com/~pavelmachek

Download attachment "signature.asc" of type "application/pgp-signature" (182 bytes)