On Mon 2019-08-12 13:53:07, Andy Shevchenko wrote:
> On Thu, Aug 08, 2019 at 03:59:36PM +0200, Stefan Roese wrote:
> > On 08.08.19 15:48, Andy Shevchenko wrote:
> > > On Thu, Aug 08, 2019 at 03:25:43PM +0200, Stefan Roese wrote:
> > > > This patch fixes a backward compatibility issue, when boards use the
> > > > old style GPIO suffix "-gpio" instead of the new "-gpios". This
> > > > potential problem has been introduced by commit d99482673f95 ("serial:
> > > > mctrl_gpio: Check if GPIO property exisits before requesting it").
> > > >
> > > > This patch now fixes this issue by iterating over all supported GPIO
> > > > suffixes by using the newly introduced for_each_gpio_suffix() helper.
> > > >
> > > > Also, the string buffer is now allocated on the stack to avoid the
> > > > problem of allocation in a loop and its potential failure.
> > >
> > > > for (i = 0; i < UART_GPIO_MAX; i++) {
> > > > enum gpiod_flags flags;
> > > > - char *gpio_str;
> > > > + const char *suffix;
> > > > + char gpio_str[32]; /* 32 is max size of property name */
> > >
> > > Hmm... don't we have some define for the maximum length of property?
> >
> > I've come up with this assumption from this code (identical comment):
> >
> > https://elixir.bootlin.com/linux/latest/source/drivers/gpio/gpiolib-of.c#L293
> >
> > (and other places in drivers/gpio/*)
>
> I tried hard to find an evidence of this in Linux kernel, I assume that comes
> from DT compiler or something, but fail. Linux kernel OF properties handling is
> written in the assumption of arbitrary length of the property name.
>
> It might be that my hard was not hard at all and I missed something.
>
> > > Or maybe we can still continue using kasprintf() approach?
> >
> > Frankly, I was feeling a bit uncomfortable with this memory allocation
> > in a loop. And Pavel also commented on this:
> >
> > https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg2066286.html
>
> If memory allocator fails, it's a big issue, and what will happen next probably
> much less important.
Not... really. With "too big" allocations, it will fail.
Anyway, my point is that allocating in a loop for this is slow and
ugly. If we don't have a maximum property length, we should probably
invent some. I mean, we can agree that 64KB property name is not okay,
right?
Pavel
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Attachment:
signature.asc
Description: Digital signature
