Re: [PATCH] pinctrl: generic: Avoid several implicit enum conversions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Sep 25, 2018 at 12:58:16PM +0200, Linus Walleij wrote:
> On Tue, Sep 25, 2018 at 8:19 AM Nathan Chancellor
> <natechancellor@xxxxxxxxx> wrote:
> 
> > Clang warns when one enumerated type is implicitly converted to another,
> > which happens several times in the pinctrl drivers for a few reasons:
> >
> > * The PCONFDUMP macro, which sets the param member in pin_config_item.
> > * The pinconf_generic_params structure, which is used by drivers to
> >   configure their bindings, which has a param member like pin_config_item.
> > * The pinconf_to_config_packed, which takes either the generic enum
> >   pin_config_param or a specialized one.
> >
> > Drivers are allowed to extend this enumerated type because of the gap
> > betweem PIN_CONFIG_END and PIN_CONFIG_MAX. Make it clear to Clang that
> > this is allowed by changing param's type in all of these instances to
> > int so no conversion needs to happen.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/138
> > Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> 
> I'm not superhappy about this because that enum is great for readability,
> even if the static syntax checker is unhappy.
> 
> If we can't have an enum here I would argue that we can just as well
> remove the enum altogether and just use #define for the config
> parameters, would you agree?
> 
> Yours,
> Linus Walleij

Hi Linus,

I see no reason to get rid of the enums. All this patch should do is
silence Clang's warnings because there is no other way to tell it that
this construct is okay except for changing the parameter/member type to
int (so no conversion needs to happen) or an explicit cast, which will
result in more noise in my opinion since this warning happens at least
10-15 times in the pinctrl drivers. There's no fundamental change to
how the enums are used.

See these other commits for similar fixes:

3eb95feac113 ("mm/zsmalloc.c: change stat type parameter to int")
04fecbf51b3c ("mm: memcontrol: use int for event/state parameter in several functions")

Cheers!
Nathan



[Index of Archives]     [Linux SPI]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux