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: <CAHp75Veb+ycdEVdSPVF7vOE3dcSNVUfPXdDcR35OCo3NPYJPCQ@mail.gmail.com>
Date: Wed, 4 Sep 2024 10:47:00 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Andy Shevchenko <andy@...nel.org>, 
	Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH v1 1/3] pinctrl: intel: Replace ifdeffery by
 pm_sleep_ptr() macro

On Wed, Sep 4, 2024 at 8:05 AM Mika Westerberg
<mika.westerberg@...ux.intel.com> wrote:
> On Tue, Sep 03, 2024 at 08:04:49PM +0300, Andy Shevchenko wrote:
> > Explicit ifdeffery is ugly and theoretically might be not synchronised
> > with the rest of functions that are assigned via pm_sleep_ptr() macro.
> > Replace ifdeffery by pm_sleep_ptr() macro to improve this.

...

> Can't we make this a stub when !PM_SLEEP?
>
> #ifdef CONFIG_PM_SLEEP
> static int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
> {
> ...
> }
> #else
> static inline int intel_pinctrl_pm_init(struct intel_pinctrl *pctrl)
> {
>         return 0;
> }
> #endif

There is no benefit. It's actually the opposite, i.e. it expands more ifdeffery.

...

> > -     ret = intel_pinctrl_pm_init(pctrl);
> > +     ret = pm_sleep_ptr(intel_pinctrl_pm_init) ? intel_pinctrl_pm_init(pctrl) : 0;
>
> Then this still looks like a function call and not like some weird
> conditional.

I understand that, but the point is to make all PM callbacks use the
same approach against kernel configuration. Current state of affairs
is simple inconsistency, but it might, however quite unlikely, lead to
desynchronization between two pm_sleep_ptr() and ifdeffery approaches.

Approach that I have before this one (and I kinda agree that ternary
here looks a bit weird) is to typedef the function and do something
like

pinctrl-intel.h:

typedef alloc_fn;

static inline int ctx_alloc(pctrl, alloc_fn)
{
  if (alloc_fn)
    return alloc_fn(pctrl);

  return 0;
}

pinctrl-intel.c:

  ret = ctx_alloc(pctrl, pm_sleep_ptr(_pm_init))
  if (ret)
    return ret;

Altogether it will be ~20+ LoCs in addition to the current codebase.

-- 
With Best Regards,
Andy Shevchenko

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ