Re: [PATCH v3 08/14] iio: bmg160_core: Simplify using devm_regulator_*get_enable()

From: Vaittinen, Matti
Date: Sat Aug 20 2022 - 02:48:59 EST


On 8/20/22 09:25, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 9:19 AM Vaittinen, Matti
> <Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:
>> On 8/20/22 02:30, Andy Shevchenko wrote:
>>> On Fri, Aug 19, 2022 at 10:21 PM Matti Vaittinen
>>> <mazziesaccount@xxxxxxxxx> wrote:
>
> What did I miss?

>>>> struct bmg160_data *data;
>>>> struct iio_dev *indio_dev;

This does already violate the rule.

>
>>> this case you even can move it out of the function, so we will see
>>> clearly that this is (not a hidden) global variable.
>>
>> Here I do disagree with you. Moving the array out of the function makes
>> it _much_ less obvious it is not used outside this function. Reason for
>> making is "static const" is to allow the data be placed in read-only
>> area (thanks to Guenter who originally gave me this tip).
>
> "static" in C language means two things (that's what come to my mind):
> - for functions this tells that a function is not used outside of the module;
> - for variables that it is a _global_ variable.
>
> Hiding static inside functions is not a good coding practice since it
> hides scope of the variable.

For const arrays the static in function does make sense. Being able to
place the data in read-only areas do help with the memory on limited
systems.

> And if you look into the kernel code, I
> believe the use you are proposing is in minority.

I don't know about the statistics. What I know is that we do have a
technical benefits when we use static const arrays instead of non static
ones in the functions. I do also believe placing the variables in blocks
is a good practice.

I tend to agree with you that using local, non const statics has
pitfalls - but the pitfalls do not really apply with const ones. You
know the value and have no races. Benefit is that just by seeing that no
pointer is returned you can be sure that no "sane code" uses the data
outside the function it resides.

Best Regards
-- Matti

--
The Linux Kernel guy at ROHM Semiconductors

Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~ this year is the year of a signature writers block ~~