linux-kernel - Re: [RESEND PATCH v2 04/13] backlight: qcom-wled: Fix off-by-one maximum with default num

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: <20211112131925.fqallv3z73ehc6y3@maple.lan>
Date:   Fri, 12 Nov 2021 13:19:25 +0000
From:   Daniel Thompson <daniel.thompson@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        phone-devel@...r.kernel.org, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Lee Jones <lee.jones@...aro.org>,
        Jingoo Han <jingoohan1@...il.com>,
        ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Pavel Dubrova <pashadubrova@...il.com>,
        Kiran Gunda <kgunda@...eaurora.org>,
        Bryan Wu <cooloney@...il.com>, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, linux-fbdev@...r.kernel.org,
        Courtney Cavin <courtney.cavin@...ymobile.com>
Subject: Re: [RESEND PATCH v2 04/13] backlight: qcom-wled: Fix off-by-one
 maximum with default num_strings

On Fri, Nov 12, 2021 at 01:35:01PM +0100, Marijn Suijten wrote:
> On 2021-11-12 12:08:39, Daniel Thompson wrote:
> > On Fri, Nov 12, 2021 at 01:26:57AM +0100, Marijn Suijten wrote:
> > > +		if (string_len > 0) {
> > > +			dev_warn(dev, "qcom,num-strings and qcom,enabled-strings are ambiguous\n");
> > 
> > The warning should also be below the error message on the next if statement.
> 
> Agreed.
> 
> > This warning occurs even when there is no ambiguity.
> > 
> > This could be:
> > 
> > 	if (string_len > 0 && val != string_len)
> > 
> > Combined these changes allows us to give a much more helpful and assertive
> > warning message:
> > 
> > qcom,num-strings mis-matches and will partially override
> > qcom,enabled-strings (remove qcom,num-strings?)
> 
> I want to let the user know it's set regardless of whether they're
> equivalent; no need to set both.
> 
> How about:
> 
>     Only one of qcom,num-strings or qcom,enabled-strings should be set
> 
> That should be more descriptive?  Otherwise, let me know if you really
> want to allow users to (unnecessarily) set both - or if it can / should
> be caught in DT validation instead.

Yes. I can live with that text. Let's use that.

Maybe I wouldn't if there gazilions of existing DTs with both
properties but IIRC the number is likely to be small or zero
(although we couldn't be 100% sure which).


Daniel.