Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 21 Jul 2023 14:31:23 +0300
From:   Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To:     Nuno =?iso-8859-1?Q?S=E1?= <noname.nuno@gmail.com>
Cc:     linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>
Subject: Re: [PATCH v1 3/8] iio: core: Switch to krealloc_array()
Message-ID: <ZLpsi8tiq4J/CUxx@smile.fi.intel.com>
References: <20230720205324.58702-1-andriy.shevchenko@linux.intel.com>
 <20230720205324.58702-4-andriy.shevchenko@linux.intel.com>
 <65dbc603062018e1cd968caf983512c2cb2039e5.camel@gmail.com>
 <ZLpaeHlJujd8l0DS@smile.fi.intel.com>
 <f565b705ba6656ae1c8b34740aa176ccfe260f60.camel@gmail.com>
 <ZLpr5KmYNlXkgBFb@smile.fi.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <ZLpr5KmYNlXkgBFb@smile.fi.intel.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Fri, Jul 21, 2023 at 02:28:36PM +0300, Andy Shevchenko wrote:
> On Fri, Jul 21, 2023 at 12:53:53PM +0200, Nuno S? wrote:
> > On Fri, 2023-07-21 at 13:14 +0300, Andy Shevchenko wrote:
> > > On Fri, Jul 21, 2023 at 09:59:37AM +0200, Nuno S? wrote:
> > > > On Thu, 2023-07-20 at 23:53 +0300, Andy Shevchenko wrote:

...

> > > > > +???????struct attribute **attrs, **attr, *clk = NULL;
> > > > > ????????struct iio_dev_attr *p;
> > > > > -???????struct attribute **attr, *clk = NULL;
> > > > > ?
> > > > > ????????/* First count elements in any existing group */
> > > > > -???????if (indio_dev->info->attrs) {
> > > > > -???????????????attr = indio_dev->info->attrs->attrs;
> > > > > -???????????????while (*attr++ != NULL)
> > > > > +???????attrs = indio_dev->info->attrs ? indio_dev->info->attrs->attrs :
> > > > > NULL;
> > > > > +???????if (attrs) {
> > > > > +???????????????for (attr = attrs; *attr; attr++)
> > > > > ????????????????????????attrcount_orig++;
> > > 
> > > > not really related with the change... maybe just mention it in the commit?
> > > 
> > > Hmm... It's related to make krealloc_array() to work as expected.
> > > 
> > 
> > Hmm, I think it's arguable :). while() -> for() it's not really needed unless
> > I'm missing something. You could even initialize 'attrs' to NULL at declaration
> > and keep the above diff minimum.
> 
> I'm not a fan of the assignments in the declarations when it potentially can be
> disrupted by a chunk of code and reading the code itself may be harder due to
> an interruption for checking the initial value. Hence, having
> 
> +	attr = attrs;
>  	while (... != NULL)
> 
> seems enough to be replaced with one liner for-loop.

Note that attrs is reused later, so the above assignment makes it cleaner that
some value is assigned to it. With the original code it's not so obvious.

> > That said, I actually prefer this style (even though some people don't like much
> > the ternary operator).
> 
> Thanks!
> 
> > > > > ????????}

-- 
With Best Regards,
Andy Shevchenko