linux-kernel - Re: [PATCH] reset: Make optional functions really optional.

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]

Date:   Fri, 23 Dec 2016 19:57:20 +0200
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
Cc:     Philipp Zabel <p.zabel@...gutronix.de>,
        linux-kernel@...r.kernel.org, CARLOS.PALMINHA@...opsys.com
Subject: Re: [PATCH] reset: Make optional functions really optional.

Hi Ramiro,

On Friday 23 Dec 2016 17:19:43 Ramiro Oliveira wrote:
> On 12/23/2016 12:08 PM, Philipp Zabel wrote:
> > Am Freitag, den 23.12.2016, 13:23 +0200 schrieb Laurent Pinchart:
> >> On Friday 23 Dec 2016 11:58:57 Philipp Zabel wrote:
> >>> Am Donnerstag, den 15.12.2016, 18:05 +0000 schrieb Ramiro Oliveira:
> >>>> Up until now optional functions in the reset API were similar to the
> >>>> non
> >>>> optional.
> >>>> 
> >>>> This patch corrects that, while maintaining compatibility with existing
> >>>> drivers.
> >>>> 
> >>>> As suggested here:
> >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lkml.org_lkml_201
> >>>> 6_12_14_502&d=DgICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=BHEb-RADEOm-lgrwdN4zqtr
> >>>> 2BWZMjeocyTkjphE6PrA&m=_0T0di-X6zgDw8ZRLDNk2ExL2EieBiCmAmuxc8OGAg4&s=H5
> >>>> BfD4P5MB85jtyUjDrn6yKu-6ws5srNWNNiFpPL0pQ&e=
> >>>> 
> >>>> Signed-off-by: Ramiro Oliveira <Ramiro.Oliveira@...opsys.com>
> >>>> ---
> >>>> 
> >>>>  drivers/reset/core.c  | 21 +++++++++++++++++++--
> >>>>  include/linux/reset.h | 46 +++++++++++++++++++++++++++++++++++++++----
> >>>>  2 files changed, 61 insertions(+), 6 deletions(-)
> >>>> 
> >>>> diff --git a/drivers/reset/core.c b/drivers/reset/core.c
> >>>> index 395dc9c..6150e7c 100644
> >>>> --- a/drivers/reset/core.c
> >>>> +++ b/drivers/reset/core.c

[snip]

> >>>>  static inline struct reset_control *reset_control_get_optional_shared(
> >>>>  					struct device *dev, const char
> >>>>  					*id)
> >>>>  {
> >>>> -	return __of_reset_control_get(dev ? dev->of_node : NULL, id,
> >>>> 0, 1);
> >>>> +
> >>>> +	struct reset_control *desc;
> >>>> +
> >>>> +	desc = __of_reset_control_get(dev ? dev->of_node : NULL, id,
> >>>> 0, 1);
> >>>> +
> >>>> +	if (IS_ERR(desc)) {
> >>>> +		if (PTR_ERR(desc) == -ENOENT)
> >>>> +			return NULL;
> >>>> +	}
> >>> 
> >>> With this duplication, I think it might be better to add an int optional
> >>> parameter
> >> 
> >> What's wrong with bool by the way ? :-)
> > 
> > Nothing wrong, it's just that the "exclusive" parameter is already int.
> > I'd be perfectly fine with using bool for both.
> 
> Do you prefer me to keep them both int, or change them to bool?

I'd prefer bool myself, it's slightly more explicit.

-- 
Regards,

Laurent Pinchart