On Tue, Jul 17 2018 13:21:31 -0600, Todd C. Miller wrote:
> > @@ -360,17 +364,15 @@ slurpit(INPUT *F)
> >                     lp->linealloc = newsize;
> >             }
> >             F->setusedc++;
> > -           memmove(lp->line, bp, len);
> > +           memmove(lp->line, line, len);
> >             lp->fpos = fpos;
> >             /* Replace trailing newline, if it exists. */
> > -           if (bp[len - 1] == '\n')
> > +           if (lp->line[len - 1] == '\n')
> >                     lp->line[len - 1] = '\0';
> > -           else
> > -                   lp->line[len] = '\0';
> > -           bp = lp->line;
> 
> 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.

Like below you mean? I'm unsure if I like it -- I wonder if it'd be
possible to use strlcpy instead of memmove here to keep things strings
and not have to deal with termination at all. But that goes beyond what
I set out to do with this diff :)

Anyway, updated as you requested so feel free to commit, or not.

diff --git a/usr.bin/join/join.c b/usr.bin/join/join.c
index 3049a423196..0ac19e5be2b 100644
--- a/usr.bin/join/join.c
+++ b/usr.bin/join/join.c
@@ -298,9 +298,9 @@ void
 slurpit(INPUT *F)
 {
        LINE *lp, *lastlp, tmp;
-       size_t len;
+       size_t len, 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 +308,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,8 +345,12 @@ 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;
+               }
+               if (line[len - 1] == '\n')
+                       len--;
                /*
                 * we depend on knowing on what field we are, one safe way is
                 * the file position.
@@ -360,17 +366,13 @@ slurpit(INPUT *F)
                        lp->linealloc = newsize;
                }
                F->setusedc++;
-               memmove(lp->line, bp, len);
+               memmove(lp->line, line, len);
                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;
+               lp->line[len - 1] = '\0';
 
                /* 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 +395,7 @@ slurpit(INPUT *F)
                        break;
                }
        }
+       free(line);
 }
 
 int

-- 
Lauri Tirkkonen | lotheac @ IRCnet

Reply via email to