| Message ID | f36e4eeee8c5bb96cddcad31a3762c10406a99e9.1670316057.git.esben@geanix.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpioset: fix memory leak in interactive mode | expand |
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
Bartosz Golaszewski <brgl@bgdev.pl> writes:
> I applied it with the change suggested by Kent.
Great, thanks.
/Esben
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; + } for (i = strlen(line) - 1; (i > 0) && isspace(line[i]); i--) line[i] = '\0';
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(-)