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

[Vaittinen, Matti]

On 8/21/22 16:08, Andy Shevchenko wrote:
> On Sat, Aug 20, 2022 at 8:27 PM Matti Vaittinen
> <mazziesaccount@xxxxxxxxx> wrote:
>> On 8/20/22 19:21, Andy Shevchenko wrote:
>>> On Sat, Aug 20, 2022 at 1:05 PM Matti Vaittinen
>>> <mazziesaccount@xxxxxxxxx> wrote:
>>>> On 8/20/22 10:18, Andy Shevchenko wrote:
>>>>> On Sat, Aug 20, 2022 at 9:48 AM Vaittinen, Matti
>>>>> <Matti.Vaittinen@xxxxxxxxxxxxxxxxx> wrote:
>>>>>> 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:

//snip

> SInce it's static it's global by nature, but local by namespace.

Which is an _improvement_ over having it in global namespace?


>> It causes no more name collisions than a regular
>> local variable does so I really don't understand your reasoning.
> 
> And I have no other words to explain it to you. You are using a global
> variable in the scope of function. This is not good for the
> maintenance and development as it's prone to get an issue in the
> future.

If you foresee some issues, please describe them as I don't see one 
single problem with a local static const data. I have seen you telling 
me that "static const" variables are _harder_ for you to review. Could 
you please explain what are the potential mistake(s) a reviewer can do, 
and what is the 'issue' that mistake can cause?

>>> So, whom should we listen to here? Because bad code is bad code. And
>>> this is code above.
>>
>> Bad is a subjective concept. I'd say the code gets much worse if we make
>> the local variable a global one.
> 
> ...
> 
> 
> To summarize, we have a huge disagreement on the placement of the
> static variables. Not sure we ever get into compromize here, so I
> leave it up to maintainers, but my opinion that it is simply a bad
> code practice.

Bad and good are labels we can place on things. We however need to have 
the reason for those labels to be meaningful. I am sorry but I don't 
want to label the local _const_ static variables bad without reason. 
This discussion starts to remind me on statements like "using goto is 
always bad" or "one must never use macros in C".

Yeah - ultimately it is a maintainer decision.

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 ~~