Re: cdio: read_track: plug leak

[Scott Cheloha]

> On Sep 10, 2017, at 9:35 PM, Michael W. Bombardieri  wrote:
> 
> Patch was also posted here, but I didn't test it.
> https://marc.info/?l=openbsd-tech=149784342025304=2

omg we're twins

Re: cdio: read_track: plug leak

[Michael W. Bombardieri]

Hi,

Patch was also posted here, but I didn't test it.
https://marc.info/?l=openbsd-tech=149784342025304=2

- Michael


On Sun, Sep 10, 2017 at 04:23:49PM -0500, Scott Cheloha wrote:
> Hi,
> 
> Saw this when preparing the monotonic clock patch.
> 
> This is a leak, right?  Every other return path in read_track()
> aside from the malloc failure frees sec.
> 
> I think the function itself is more confusing than it needs to be
> and could use a refactor but that belongs in a separate patch.
> 
> Feedback?
> 
> --
> Scott Cheloha
> 
> Index: usr.bin/cdio/rip.c
> ===
> RCS file: /cvs/src/usr.bin/cdio/rip.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 rip.c
> --- usr.bin/cdio/rip.c20 Aug 2015 22:32:41 -  1.16
> +++ usr.bin/cdio/rip.c10 Sep 2017 21:16:51 -
> @@ -398,6 +398,7 @@ read_track(struct track *ti)
>   }
>   if (ti->hdl != NULL &&
>   (sio_write(ti->hdl, sec, blksize) == 0)) {
> + free(sec);
>   sio_close(ti->hdl);
>   ti->hdl = NULL;
>   warnx("\nerror while writing to audio output");
> 

cdio: read_track: plug leak

[Scott Cheloha]

Hi,

Saw this when preparing the monotonic clock patch.

This is a leak, right?  Every other return path in read_track()
aside from the malloc failure frees sec.

I think the function itself is more confusing than it needs to be
and could use a refactor but that belongs in a separate patch.

Feedback?

--
Scott Cheloha

Index: usr.bin/cdio/rip.c
===
RCS file: /cvs/src/usr.bin/cdio/rip.c,v
retrieving revision 1.16
diff -u -p -r1.16 rip.c
--- usr.bin/cdio/rip.c  20 Aug 2015 22:32:41 -  1.16
+++ usr.bin/cdio/rip.c  10 Sep 2017 21:16:51 -
@@ -398,6 +398,7 @@ read_track(struct track *ti)
}
if (ti->hdl != NULL &&
(sio_write(ti->hdl, sec, blksize) == 0)) {
+   free(sec);
sio_close(ti->hdl);
ti->hdl = NULL;
warnx("\nerror while writing to audio output");