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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210719120523.GA20484@duo.ucw.cz>
Date:   Mon, 19 Jul 2021 14:05:23 +0200
From:   Pavel Machek <pavel@....cz>
To:     Randy Dunlap <rdunlap@...radead.org>
Cc:     Amy Parker <apark0006@...dent.cerritos.edu>,
        kernel test robot <lkp@...el.com>, kbuild-all@...ts.01.org,
        linux-leds@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] swap led_brightness from enum to typedef

Hi!

> >>> vim +18 include/media/v4l2-flash-led-class.h
> >>>
> >>>     14
> >>>     15  struct led_classdev_flash;
> >>>     16  struct led_classdev;
> >>>     17  struct v4l2_flash;
> >>>   > 18  led_brightness;
> >>>     19
> >>>
> >>> ---
> 
> > 
> > Another patch was sent into the list to correct this error.
> 
> Hopefully Pavel (LED subsystem maintainer) will comment soon-ish.
> 
> My comments:
> 
> a. This patch would be the right thing to do if your large patch had already been
> applied (merged) somewhere, but AFAIK it hasn't been. So:
> 
> b. IMO you should resend your entire patch set with this fix included.
> Send it as "v2" (version 2) and explain the changes in it since your
> original patch (which was v1). This v2 explanation should be below the
> "---" line in the patch. (See Documentation/process/submitting-patches.rst
> for more info -- or ask for more info/help.)

I still remember the old patch, so b. is not strictly neccessary here.

> c. For your follow-up patch to include/media/v4l2-flash-led-class.h, which was:
> 
> -led_brightness;
> +typedef u8 led_brightness;
> 
> I would just add this to include/media/v4l2-flash-led-class.h:
> 
> #include <linux/leds.h>
> 
> That way, in a few years, when the type of led_brightness changes again,
> someone won't have to remember to search for other typedefs of it and
> update them also. Or maybe they will do that after a bug happens and
> someone notices it.
> 
> (Note that I am just trying to help. Pavel has more of a final
> say-so about this.)

And your comments are reasonable.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

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