Re: tail: -r mem leak with non-regular files

[Tobias Stoeckmann]

On Thu, Mar 26, 2015 at 11:41:23PM +0100, Tobias Stoeckmann wrote:
> The less obvious one is in an error path.

As tl->l is always of fixed size (BSZ), we can just change the struct
to have a BSZ sized array in it. This removes the need to do checks
in the error path completely.

While at it, there is no use to have a typedef in the code, so I just
removed it, too.

Index: reverse.c
===================================================================
RCS file: /cvs/src/usr.bin/tail/reverse.c,v
retrieving revision 1.19
diff -u -p -r1.19 reverse.c
--- reverse.c   27 Oct 2009 23:59:44 -0000      1.19
+++ reverse.c   27 Mar 2015 11:19:14 -0000
@@ -147,12 +147,13 @@ r_reg(FILE *fp, enum STYLE style, off_t 
        return (0);
 }
 
-typedef struct bf {
+#define        BSZ     (128 * 1024)
+struct bf {
        struct bf *next;
        struct bf *prev;
        size_t len;
-       char *l;
-} BF;
+       char l[BSZ];
+};
 
 /*
  * r_buf -- display a non-regular file in reverse order by line.
@@ -167,21 +168,19 @@ typedef struct bf {
 static void
 r_buf(FILE *fp)
 {
-       BF *mark, *tr, *tl = NULL;
+       struct bf *mark, *tr, *tl = NULL;
        int ch;
        size_t len, llen;
        char *p;
        off_t enomem;
 
-#define        BSZ     (128 * 1024)
        for (mark = NULL, enomem = 0;;) {
                /*
                 * Allocate a new block and link it into place in a doubly
                 * linked list.  If out of memory, toss the LRU block and
                 * keep going.
                 */
-               if (enomem || (tl = malloc(sizeof(BF))) == NULL ||
-                   (tl->l = malloc(BSZ)) == NULL) {
+               if (enomem || (tl = malloc(sizeof(*tl))) == NULL) {
                        if (!mark)
                                err(1, NULL);
                        tl = enomem ? tl->next : mark;
@@ -259,5 +258,12 @@ r_buf(FILE *fp)
        while ((tl = tl->next)->len) {
                WR(tl->l, tl->len);
                tl->len = 0;
+       }
+
+       tl->prev->next = NULL;
+       while (tl != NULL) {
+               tr = tl->next;
+               free(tl);
+               tl = tr;
        }
 }