Re: uniq(1) is broken

[Rin Okuyama]

On 2016/10/16 15:18, Abhinav Upadhyay wrote:

Your patch looks good, I tested and committed. Thanks for the fix :-)


Fix confirmed. Thanks for your review and commit.

Rin

Re: uniq(1) is broken

[Abhinav Upadhyay]

On Sun, Oct 16, 2016 at 6:58 AM, Rin Okuyama
<rokuy...@rk.phys.keio.ac.jp> wrote:
> uniq(1) is currently broken:
>
>   % cat test
>   1
>   12
>   1
>   1
>   % uniq test
>   1
>   12
>   1
>   1
>
> This bug was introduced to uniq.c rev. 1.19,
>
>   http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.bin/uniq/uniq.c#rev1.19
>

Your patch looks good, I tested and committed. Thanks for the fix :-)

-
Abhinav

uniq(1) is broken

[Rin Okuyama]

uniq(1) is currently broken:

  % cat test
  1
  12
  1
  1
  % uniq test
  1
  12
  1
  1

This bug was introduced to uniq.c rev. 1.19,

  http://cvsweb.netbsd.org/bsdweb.cgi/src/usr.bin/uniq/uniq.c#rev1.19

where the size of buffer and the length of line are mixed up.
Please apply the attached patch. Beside fixing this bug, a tiny
optimization has been made; do not call skip() twice per line.

Thanks,
Rin

--- src/usr.bin/uniq/uniq.c.orig2016-10-16 08:11:18.115254706 +0900
+++ src/usr.bin/uniq/uniq.c 2016-10-16 09:33:36.693367693 +0900
@@ -65,12 +65,12 @@
 int
 main (int argc, char *argv[])
 {
-   const char *t1, *t2;
+   const char *prevp, *thisp;
FILE *ifp, *ofp;
int ch;
char *prevline, *thisline, *p;
size_t prevlinesize, thislinesize, psize;
-   size_t prevlinecompsize, thislinecompsize;
+   size_t prevlen, thislen;
 
 	setprogname(argv[0]);

ifp = ofp = NULL;
@@ -127,11 +127,15 @@
 
 	if ((p = fgetln(ifp, )) == NULL)

return 0;
-   prevlinesize = psize;
+   prevlinesize = prevlen = psize;
if ((prevline = malloc(prevlinesize + 1)) == NULL)
err(1, "malloc");
(void)memcpy(prevline, p, prevlinesize);
prevline[prevlinesize] = '\0';
+   if (numfields || numchars)
+   prevp = skip(prevline, );
+   else
+   prevp = prevline;
 
 	thislinesize = psize;

if ((thisline = malloc(thislinesize + 1)) == NULL)
@@ -143,22 +147,18 @@
err(1, "realloc");
thislinesize = psize;
}
+   thislen = psize;
(void)memcpy(thisline, p, psize);
thisline[psize] = '\0';
-   thislinecompsize = thislinesize;
-   prevlinecompsize = prevlinesize;
 
 		/* If requested get the chosen fields + character offsets. */

-   if (numfields || numchars) {
-   t1 = skip(thisline, );
-   t2 = skip(prevline, );
-   } else {
-   t1 = thisline;
-   t2 = prevline;
-   }
+   if (numfields || numchars)
+   thisp = skip(thisline, );
+   else
+   thisp = thisline;
 
 		/* If different, print; set previous to new value. */

-   if (thislinecompsize != prevlinecompsize || strcmp(t1, t2)) {
+   if (thislen != prevlen || strcmp(thisp, prevp)) {
char *t;
size_t ts;
 
@@ -169,6 +169,8 @@

ts = prevlinesize;
prevlinesize = thislinesize;
thislinesize = ts;
+   prevp = thisp;
+   prevlen = thislen;
repeats = 0;
} else
++repeats;