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

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Matti Vaittinen <mazziesaccount@gmail.com>
Cc: "Jonathan Cameron" <jic23@kernel.org>,
	"Matti Vaittinen" <matti.vaittinen@fi.rohmeurope.com>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	linux-iio <linux-iio@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 07/14] iio: ltc2688: Simplify using devm_regulator_*get_enable()
Date: Sun, 21 Aug 2022 16:13:11 +0300	[thread overview]
Message-ID: <CAHp75VchPCHsBcx7mMoGUjz=s4hmfnO6t7DqtpWfg=aGrbo1Fg@mail.gmail.com> (raw)
In-Reply-To: <103abfae-6c0d-9a2e-2d59-0da4c8be3eb4@gmail.com>

On Sat, Aug 20, 2022 at 10:00 PM Matti Vaittinen
<mazziesaccount@gmail.com> wrote:
> On 8/20/22 20:41, Andy Shevchenko wrote:
> > On Sat, Aug 20, 2022 at 8:30 PM Matti Vaittinen
> > <mazziesaccount@gmail.com> wrote:
> >> On 8/20/22 19:09, Andy Shevchenko wrote:
> >>> On Sat, Aug 20, 2022 at 4:45 PM Matti Vaittinen
> >>> <mazziesaccount@gmail.com> wrote:
> >>>> On 8/20/22 14:21, Jonathan Cameron wrote:
> >>>>> On Fri, 19 Aug 2022 22:19:17 +0300
> >>>>> Matti Vaittinen <mazziesaccount@gmail.com> wrote:
> >
> > ...
> >
> >>>>> For the whole static / vs non static. My personal preference is not
> >>>>> to have the static marking but I don't care that much.
> >>>>
> >>>> I'd like to stick with the static here. I know this one particular array
> >>>> does not have much of a footprint - but I'd like to encourage the habit
> >>>> of considering the memory usage. This discussion serves as an example of
> >>>> how unknown the impact of making const data static is. I didn't know
> >>>> this myself until Sebastian educated me :)  Hence my strong preference
> >>>> on keeping this 'static' as an example for others who are as ignorant as
> >>>> I were ;) After all, having const data arrays static is quite an easy
> >>>> way of improving things - and it really does matter when there is many
> >>>> of arrays - or when they contain large data.
> >>>
> >>> But still the same comment about global scope of the variable is applied.
> >>
> >> I don't understand why you keep claiming the variable is global when it
> >> is not?
> >
> > It is. The static keyword makes it global, but putting the entire
> > definition into the function is asking for troubles.

> Please, describe the trouble we can get with a local static const
> variable? A real concrete threat there is. I have explained the benefit.
> I have also explained the concrete possibility of name collision when we
> really do a global out of local.

I told you, the benefit is not to play dirty tricks on developers,
maintainers and reviewers. It's simply harder to read the code and get
the usage of the variable that lifetime is out of scope of the
function.

> > I guess some C standard chapter describes that in non-understandable language.
> >
> >>> As I explained before, hiding global variables inside a function is a
> >>> bad code practice.
> >>
> >> I don't really get what you mean here. And I definitely don't see any
> >> improvement if we would really use a global variable instead of a local one.
> >
> > The improvement is avoid hiding the global variable to the local namespace.
>
> I guess you mean that you may miss the fact that a variable stays there
> even after execution exits the function, right? Ok, let's assume someone
> misses this point when reading the code. Now, please describe me the
> potential issues this can cause knowing our static is const and doesn't
> change the value.

When you hide the static variable inside the function, you simply
narrow visibility to the compiler, but the variable stays all the time
the module is in.

-- 
With Best Regards,
Andy Shevchenko