gpioset: fix memory leak in interactive mode

Even when readline() returns an empty buffer, we still need to free() it to
avoid leaking memory.

Signed-off-by: Esben Haabendal <esben@geanix.com>
---
 tools/gpioset.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

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@geanix.com>

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
>

On Wed, Dec 7, 2022 at 10:28 AM Kent Gibson <warthog618@gmail.com> 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@geanix.com>
>
> 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