Re: [PATCH] gpioset: fix memory leak in interactive mode

[Bartosz Golaszewski <brgl@xxxxxxxx>]

On Wed, Dec 7, 2022 at 10:28 AM Kent Gibson <warthog618@xxxxxxxxx> wrote:
>
> On Tue, Dec 06, 2022 at 09:46:37AM +0100, Esben Haabendal wrote:
> > Even when readline() returns an empty buffer, we still need to free() it to
> > avoid leaking memory.
> >
>
> Good point.
>
> > Signed-off-by: Esben Haabendal <esben@xxxxxxxxxx>
>
> As per the README, you should prefix your subject with [libgpiod] to
> better get the attention of the libgpiod team.
>
> > ---
> >  tools/gpioset.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/gpioset.c b/tools/gpioset.c
> > index c49d229870d2..f087003af1c9 100644
> > --- a/tools/gpioset.c
> > +++ b/tools/gpioset.c
> > @@ -768,8 +768,12 @@ static void interact(struct gpiod_line_request **requests,
> >               fflush(stdout);
> >
> >               line = readline(PROMPT);
> > -             if (!line || line[0] == '\0')
> > +             if (!line)
> >                       continue;
> > +             if (line[0] == '\0') {
> > +                     free(line);
> > +                     continue;
> > +             }
> >
>
> Given free() is null-aware, I would just add the free before the
> continue, rather than splitting out the cases, so
>
>                 line = readline(PROMPT);
> -               if (!line || line[0] == '\0')
> +               if (!line || line[0] == '\0') {
> +                       free(line);
>                         continue;
> +               }
>
>
> Cheers,
> Kent.
>
> >               for (i = strlen(line) - 1; (i > 0) && isspace(line[i]); i--)
> >                       line[i] = '\0';
> > --
> > 2.38.1
> >

I applied it with the change suggested by Kent.

Bart