Hi Todd,
Todd C. Miller wrote on Tue, Jul 17, 2018 at 01:38:14PM -0600:
> On Tue, 17 Jul 2018 13:21:31 -0600, "Todd C. Miller" wrote:
>> It probably makes more sense to do the newline check (and decrement
>> len if one is present) before newsize is computed. Then you would
>> need to unconditionally NUL-terminate lp->line.
> Perhaps something like this.
OK schwarze@
I think it's more elegant to just break; when getline() returns -1,
you already have the free() at the end:
if ((len = getline(&line, &linesize, F->fp)) == -1)
break;
But you get to choose the colour of the bikeshed, OK either way.
Yours,
Ingo
> Index: usr.bin/join/join.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/join/join.c,v
> retrieving revision 1.27
> diff -u -p -u -r1.27 join.c
> --- usr.bin/join/join.c 9 Oct 2015 01:37:07 -0000 1.27
> +++ usr.bin/join/join.c 17 Jul 2018 19:33:38 -0000
> @@ -298,9 +298,10 @@ void
> slurpit(INPUT *F)
> {
> LINE *lp, *lastlp, tmp;
> - size_t len;
> + ssize_t len;
> + size_t linesize;
> u_long cnt;
> - char *bp, *fieldp;
> + char *bp, *fieldp, *line;
> long fpos;
> /*
> * Read all of the lines from an input file that have the same
> @@ -308,6 +309,8 @@ slurpit(INPUT *F)
> */
>
> F->setcnt = 0;
> + line = NULL;
> + linesize = 0;
> for (lastlp = NULL; ; ++F->setcnt, lastlp = lp) {
> /*
> * If we're out of space to hold line structures, allocate
> @@ -343,13 +346,19 @@ slurpit(INPUT *F)
> F->pushbool = 0;
> continue;
> }
> - if ((bp = fgetln(F->fp, &len)) == NULL)
> + if ((len = getline(&line, &linesize, F->fp)) == -1) {
> + free(line);
> return;
> + }
> /*
> * we depend on knowing on what field we are, one safe way is
> * the file position.
> */
> fpos = ftell(F->fp) - len;
> +
> + /* Remove trailing newline, if it exists, and copy line. */
> + if (line[len - 1] == '\n')
> + len--;
> if (lp->linealloc <= len + 1) {
> char *p;
> u_long newsize = lp->linealloc +
> @@ -360,17 +369,13 @@ slurpit(INPUT *F)
> lp->linealloc = newsize;
> }
> F->setusedc++;
> - memmove(lp->line, bp, len);
> + memcpy(lp->line, line, len);
> + lp->line[len] = '\0';
> lp->fpos = fpos;
> - /* Replace trailing newline, if it exists. */
> - if (bp[len - 1] == '\n')
> - lp->line[len - 1] = '\0';
> - else
> - lp->line[len] = '\0';
> - bp = lp->line;
>
> /* Split the line into fields, allocate space as necessary. */
> lp->fieldcnt = 0;
> + bp = lp->line;
> while ((fieldp = strsep(&bp, tabchar)) != NULL) {
> if (spans && *fieldp == '\0')
> continue;
> @@ -393,6 +398,7 @@ slurpit(INPUT *F)
> break;
> }
> }
> + free(line);
> }
>
> int
