svn commit: r311895 - in head: etc/mtree usr.bin/tail usr.bin/tail/tests

[Alan Somers]

On Wed, Jan 11, 2017 at 5:53 AM, Sergey Kandaurov <pluknet at gmail.com> wrote:
> On 10 January 2017 at 23:43, Alan Somers <asomers at freebsd.org> wrote:
>>
>> Author: asomers
>> Date: Tue Jan 10 20:43:32 2017
>> New Revision: 311895
>> URL: https://svnweb.freebsd.org/changeset/base/311895
>>
>> Log:
>>   Fix memory leaks during "tail -r" of an irregular file
>> [..]
>>
>> -typedef struct bf {
>> -       struct bf *next;
>> -       struct bf *prev;
>> -       int len;
>> -       char *l;
>> -} BF;
>> +static const size_t bsz = 128 * 1024;
>> +typedef struct bfelem {
>> +       TAILQ_ENTRY(bfelem) entries;
>> +       size_t len;
>> +       char l[bsz];
>> +} bfelem_t;
>>
>
> This breaks on gcc that doesn't respect const for some reason:
> reverse.c:177: error: variably modified 'l' at file scope
>
>>
>>  /*
>>   * r_buf -- display a non-regular file in reverse order by line.
>> @@ -189,64 +190,42 @@ typedef struct bf {
>>  static void
>>  r_buf(FILE *fp, const char *fn)
>>  {
>> -       BF *mark, *tl, *tr;
>> -       int ch, len, llen;
>> +       struct bfelem *tl, *temp, *first = NULL;
>> +       size_t len, llen;
>
>
> reverse.c:194: warning: 'len' may be used uninitialized in this function
>
> I suspect this is due to a typo on line 254.
>
>  [..]
>
>>
>>         /*
>> -        * Step through the blocks in the reverse order read.  The last
>> char
>> -        * is special, ignore whether newline or not.
>> +        * Now print the lines in reverse order
>> +        * Outline:
>> +        *    Scan backward for "\n",
>> +        *    print forward to the end of the buffers
>> +        *    free any buffers that start after the "\n" just found
>> +        *    Loop
>>          */
>> -       for (mark = tl;;) {
>> -               for (p = tl->l + (len = tl->len) - 1, llen = 0; len--;
>> -                   --p, ++llen)
>> -                       if (*p == '\n') {
>> -                               if (llen) {
>> +       tl = TAILQ_LAST(&head, bfhead);
>> +       first = TAILQ_FIRST(&head);
>> +       while (tl != NULL) {
>> +               for (p = tl->l + tl->len - 1, llen = 0; p >= tl->l;
>> +                   --p, ++llen) {
>> +                       int start = (tl == first && p == tl->l);
>> +
>> +                       if ((*p == '\n') || start) {
>> +                               struct bfelem *tr;
>> +
>> +                               if (start && len)
>
>
> here
>
> joint patch to fix build on gcc (not tested, although tests pass)
>
> Index: reverse.c
> ===================================================================
> --- reverse.c    (revision 311927)
> +++ reverse.c    (working copy)
> @@ -170,11 +170,11 @@
>          ierr(fn);
>  }
>
> -static const size_t bsz = 128 * 1024;
> +#define    BSZ    (128 * 1024)
>  typedef struct bfelem {
>      TAILQ_ENTRY(bfelem) entries;
>      size_t len;
> -    char l[bsz];
> +    char l[BSZ];
>  } bfelem_t;
>
>  /*
> @@ -216,9 +216,9 @@
>
>          /* Fill the block with input data. */
>          len = 0;
> -        while ((!feof(fp)) && len < bsz) {
> +        while ((!feof(fp)) && len < BSZ) {
>              p = tl->l + len;
> -            len += fread(p, 1, bsz - len, fp);
> +            len += fread(p, 1, BSZ - len, fp);
>              if (ferror(fp)) {
>                  ierr(fn);
>                  return;
> @@ -251,7 +251,7 @@
>              if ((*p == '\n') || start) {
>                  struct bfelem *tr;
>
> -                if (start && len)
> +                if (start && llen)
>                      WR(p, llen + 1);
>                  else if (llen)
>                      WR(p + 1, llen);
>
> --
> wbr,
> pluknet

Thanks for the tip, Sergey.  Fixed in 311928.