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: <CAMRc=Me-FMZ3e=EaUA1kimEonz=HVHBp7coxCz53bJK9NYBuFg@mail.gmail.com>
Date:   Thu, 9 Mar 2023 16:22:19 +0100
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Schspa Shi <schspa@...il.com>, Marc Zyngier <maz@...nel.org>,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, linux-pwm@...r.kernel.org,
        linux-stm32@...md-mailman.stormreply.com,
        patches@...nsource.cirrus.com,
        Linus Walleij <linus.walleij@...aro.org>,
        Doug Berger <opendmb@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Andy Shevchenko <andy@...nel.org>,
        Thierry Reding <thierry.reding@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Nandor Han <nandor.han@...com>,
        Semi Malinen <semi.malinen@...com>
Subject: Re: [PATCH v1 00/16] gpio: Use string_choices.h

On Mon, Mar 6, 2023 at 8:55 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> Use string_choices.h in the GPIO drivers and library.
> It has been tested on x86_64 and (semi-)compile tested
> over all.
>
> Andy Shevchenko (16):
>   lib/string_helpers: Add missing header files to MAINTAINERS database
>   lib/string_helpers: Split out string_choices.h
>   lib/string_choices: Add str_high_low() helper
>   lib/string_choices: Add str_input_output() helper
>   gpiolib: Utilize helpers from string_choices.h
>   gpio: adnp: Utilize helpers from string_choices.h
>   gpio: brcmstb: Utilize helpers from string_choices.h
>   gpio: crystalcove: Utilize helpers from string_choices.h
>   gpio: grgpio: Utilize helpers from string_choices.h
>   gpio: mvebu: Utilize helpers from string_choices.h
>   gpio: pl061: Utilize helpers from string_choices.h
>   gpio: stmpe: Utilize helpers from string_choices.h
>   gpio: wcove: Utilize helpers from string_choices.h
>   gpio: wm831x: Utilize helpers from string_choices.h
>   gpio: wm8994: Utilize helpers from string_choices.h
>   gpio: xra1403: Utilize helpers from string_choices.h
>
>  MAINTAINERS                     |  3 ++
>  drivers/gpio/gpio-adnp.c        | 24 ++++----------
>  drivers/gpio/gpio-brcmstb.c     |  3 +-
>  drivers/gpio/gpio-crystalcove.c | 17 +++++-----
>  drivers/gpio/gpio-grgpio.c      |  3 +-
>  drivers/gpio/gpio-mvebu.c       | 27 +++++++---------
>  drivers/gpio/gpio-pl061.c       |  4 +--
>  drivers/gpio/gpio-stmpe.c       | 19 +++++------
>  drivers/gpio/gpio-wcove.c       | 15 ++++-----
>  drivers/gpio/gpio-wm831x.c      |  5 +--
>  drivers/gpio/gpio-wm8994.c      |  6 ++--
>  drivers/gpio/gpio-xra1403.c     |  5 +--
>  drivers/gpio/gpiolib-sysfs.c    |  3 +-
>  drivers/gpio/gpiolib.c          | 13 ++++----
>  include/linux/string_choices.h  | 56 +++++++++++++++++++++++++++++++++
>  include/linux/string_helpers.h  | 26 +--------------
>  16 files changed, 125 insertions(+), 104 deletions(-)
>  create mode 100644 include/linux/string_choices.h
>
> --
> 2.39.1
>

I've been thinking about this and I must say it doesn't make much
sense to me. Not only does it NOT reduce the code size (even if we
assume the unlikely case where we'd build all those modules that use
the helpers) but also decreases the readability for anyone not
familiar with the new interfaces (meaning time spent looking up the
new function). The "%s", x ? "if" : "else" statement is concise and
clear already, I don't see much improvement with this series. And I'm
saying it from the position of someone who loves factoring out common
code. :)

I'll wait to hear what others have to say but if it were up to me, I'd
politely say no.

(I mean: I guess, in the end it is up to me, but I'm open to arguments.) :)

Bart

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ