> On Sep 1, 2023, at 3:48 AM, Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx> wrote: > > On Thu, Aug 31, 2023 at 11:20:39AM -0500, Vlad Karpovich wrote: >> From: Ricardo Rivera-Matos <rriveram@xxxxxxxxxxxxxxxxxxxxx> >> >> Checks the index computed by the virq offset before printing the >> error condition in cs35l45_spk_safe_err() handler. >> >> Signed-off-by: Ricardo Rivera-Matos <rriveram@xxxxxxxxxxxxxxxxxxxxx> >> Signed-off-by: Vlad Karpovich <vkarpovi@xxxxxxxxxxxxxxxxxxxxx> >> --- >> sound/soc/codecs/cs35l45.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/sound/soc/codecs/cs35l45.c b/sound/soc/codecs/cs35l45.c >> index 2ac4612402eb..02b1172d2647 100644 >> --- a/sound/soc/codecs/cs35l45.c >> +++ b/sound/soc/codecs/cs35l45.c >> @@ -1023,7 +1023,10 @@ static irqreturn_t cs35l45_spk_safe_err(int irq, void *data) >> >> i = irq - regmap_irq_get_virq(cs35l45->irq_data, 0); >> >> - dev_err(cs35l45->dev, "%s condition detected!\n", cs35l45_irqs[i].name); >> + if (i < 0 || i >= ARRAY_SIZE(cs35l45_irqs)) > > I am happy enough for this to be merged, since it clearly does > no harm. So: > > Acked-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxx> > > But I do still have a slight reservation of what is the point > of this error check? This handler is static and can only be > called from within cs35l45.c and the only code that registers > IRQs goes through the cs35l45_irqs array and registers IRQs > from there, so how does this ever end up with i being out of > bounds? > > And whilst I would not add this to this patch. I do also think > perhaps Ricardo had a point in his email, the IRQ handler > should probably be renamed, since it handles more than just > the spk_safe_err's, perhaps something like cs35l45_report_err. > On why the watchdog and global error call this as well, that > was a review comment on an earlier patch since the handlers for > those errors only printed a message, they might as well be > combined with the spk_safe error that also only printed a > message. If at some point separate handling is added for them > they can be split out. Thanks Charles, I had missed that comment. It’s clear to me now. > > Thanks, > Charles Acked-by: Ricardo Rivera-Matos <rriveram@xxxxxxxxxxxxxxxxxxxxx <mailto:rriveram@xxxxxxxxxxxxxxxxxxxxx>>
