Re: [PATCH v1] driver core: Annotate dev_err_probe() with __must_check

From: Krzysztof Kozlowski
Date: Wed Sep 09 2020 - 03:08:33 EST


On Wed, 9 Sep 2020 at 09:02, Greg Kroah-Hartman
<gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Sep 09, 2020 at 08:29:25AM +0200, Krzysztof Kozlowski wrote:
> > On Wed, 26 Aug 2020 at 18:18, Joe Perches <joe@xxxxxxxxxxx> wrote:
> > >
> > > On Wed, 2020-08-26 at 18:55 +0300, Andy Shevchenko wrote:
> > > > On Wed, Aug 26, 2020 at 08:44:30AM -0700, Joe Perches wrote:
> > > > > On Wed, 2020-08-26 at 13:44 +0300, Andy Shevchenko wrote:
> > > >
> > > > ...
> > > >
> > > > > > -int dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> > > > > > +int __must_check dev_err_probe(const struct device *dev, int err, const char *fmt, ...);
> >
> > +Cc Stephen and Greg,
> >
> > Hi Andy,
> >
> > Did this patch ended up in next somehow? I am surprised because now I
> > got warnings for perfectly fine code:
> > https://lore.kernel.org/linux-next/20200909155654.76fe3bd6@xxxxxxxxxxxxxxxx/T/#u
> >
> > This creates simply false warnings instead of hints for "optimization".
>
> Yes, it got merged into m y driver core tree.
>
> I'll fix up the tty build warning, should be easy enough, the patch is
> below.

Yes, this fix suppresses the warning but the question is whether we
really want the warning?
Such fixes mean additional code which the compiler might not optimize
(unless it inlines the dev_err_probe()). This additional code is
purely for suppressing the warning, without any meaning on its own.
Actually it might be even confusing for someone to see:
if (ret)
ret = dev_err_probe(ret);

warn_unused_result should point errors, not "optimization
opportunities". If you want to have opportunity, add a coccinelle
rule. Or a checkpatch rule. Not a compiler warning.

Best regards,
Krzysztof