On Wed, 22 Jul 2020 20:29:06 +0200, Martijn van Duren wrote:
> When r1.28 converted join(1) to getline(3) it left the old intermediate
> line variable. This means using an additional memcpy and reallocing.
> I reckon this is wasted cycles and screen filling.
>
> Major difference is that our getline allocates to the next power of 2,
> while the current code overcommits a maximum of ~100 bytes. I reckon
> this is a fair trade-off.
Looks good, one minor comment inline. Either way, OK millert@
- todd
> Index: join.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/join/join.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 join.c
> --- join.c 14 Nov 2018 15:16:09 -0000 1.32
> +++ join.c 22 Jul 2020 18:19:34 -0000
> @@ -271,9 +271,8 @@ slurp(INPUT *F)
> {
> LINE *lp, *lastlp, tmp;
> ssize_t len;
> - size_t linesize;
> u_long cnt;
> - char *bp, *fieldp, *line;
> + char *bp, *fieldp;
>
> /*
> * Read all of the lines from an input file that have the same
> @@ -281,8 +280,6 @@ slurp(INPUT *F)
> */
>
> F->setcnt = 0;
> - line = NULL;
> - linesize = 0;
> for (lastlp = NULL; ; ++F->setcnt) {
> /*
> * If we're out of space to hold line structures, allocate
> @@ -320,22 +317,12 @@ slurp(INPUT *F)
> F->pushbool = 0;
> continue;
> }
> - if ((len = getline(&line, &linesize, F->fp)) == -1)
> + if ((len = getline(&(lp->line), &(lp->linealloc), F->fp)) == -1
> )
> break;
>
> /* Remove trailing newline, if it exists, and copy line. */
> - if (line[len - 1] == '\n')
> + if (lp->line[len - 1] == '\n')
> len--;
You could replace "len--" with "lp->line[len - 1] = '\0'" here (or
"lp->line[--len] = '\0'" if you prefer but len is otherwise unused).
> - if (lp->linealloc <= len + 1) {
> - char *p;
> - u_long newsize = lp->linealloc +
> - MAXIMUM(100, len + 1 - lp->linealloc);
> - if ((p = realloc(lp->line, newsize)) == NULL)
> - err(1, NULL);
> - lp->line = p;
> - lp->linealloc = newsize;
> - }
> - memcpy(lp->line, line, len);
> lp->line[len] = '\0';
Then there would be no need to NUL terminate here since getline(3)
always NUL terminates.
>
> /* Split the line into fields, allocate space as necessary. */
> @@ -363,7 +350,6 @@ slurp(INPUT *F)
> break;
> }
> }
> - free(line);
> }
>
> char *
