Re: [PATCH v2 6/7] gpiolib: avoid krealloc() in gpiod_get_array()

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Nov 04, 2026 at 03:23:26PM +0100, Bartosz Golaszewski wrote:
> On Wed, Nov 04, 2026 at 03:48:34AM +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> > - [Low] gpiolib: delayed initialization of __free() variable in gpiod_get_array()
> > --
> > 
> > commit ab11568406b8d4aa5255ce95e09509f3e6c86285
> > Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > 
> > gpiolib: avoid krealloc() in gpiod_get_array()
> > 
> > This commit defers allocating the gpio_descs structure in gpiod_get_array()
> > until the first GPIO descriptor is retrieved so that it can be allocated in
> > a single pass without needing krealloc(). It also factors descriptor
> > allocation and fast-path checks into helper functions.
> >
> 
> ...
> 
> > > +			descs = gpio_descs_alloc(count, desc);
> > > +			if (!descs)
> > >  				return ERR_PTR(-ENOMEM);
> > > -
> > > -			descs = array;
> > > -
> > > -			array_info = (void *)descs + descs_size;
> > > -			array_info->get_mask = array_info->invert_mask +
> > > -						  bitmap_size;
> > > -			array_info->set_mask = array_info->get_mask +
> > > -						  bitmap_size;
> > > -
> > > -			array_info->desc = descs->desc;
> > > -			array_info->size = count;
> > > -			array_info->gdev = gdev;
> > > -			bitmap_set(array_info->get_mask, 0, count);
> > > -			bitmap_set(array_info->set_mask, 0, count);
> > > -			descs->info = array_info;
> > >  		}
> > 
> > [Severity: Low]
> > This isn't a bug, but in gpiod_get_array(), descs is declared at the top of
> > the function with __free(gpiod_put_array) = NULL and then assigned later
> > inside the loop when i == 0.
> > 
> > The cleanup.h conventions ask that __free() variables be defined and
> > initialized in a single statement at the point of resource acquisition
> > rather than initialized to NULL at the top of the function and assigned
> > later. Could the first descriptor be fetched before the loop, or the
> > allocation structured so that descs is defined and initialized in one
> > statement?
> >
> 
> Hi Dmitry, please follow the cleanup.h conventions here and in the
> subsequent patch.

Hi Bart,

The convention (combining declaration with allocation) will not work
here because that would make the allocation have wrong scope.

If you want I can move declaration + NULL initialization just before the
loop. The topic of pulling allocation out of the loop was discussed with
Andy, it would result in duplicating the code and making the result much
worse.

Thanks.

-- 
Dmitry