Hi,
Theo Buehler wrote on Mon, Apr 02, 2018 at 08:32:43PM +0200:
> On Mon, Apr 02, 2018 at 07:41:55PM +0200, Stefan Sperling wrote:
>> Line buffers allocated in preadline() were never freed.
> They are freed in ignoreline().
What a beautiful case of obfuscation. How symmetrical. Both
the malloc(3) and the free(3) are hidden away in a function.
That said, the surrounding code looks correct, but quite strange.
OK to clean it up a bit with the patch below?
(The loop bodies do not change, they are just reindented,
as you can check with patch && cvs diff -Nub)
Yours,
Ingo
Rationale:
* if (a <= b) for (i = a; i <= b;
doas not make sense to me.
If a > b, we have i > b right away, so the loop isn't entered.
* a > b ||
does not make sense either, for the same reason:
The for loop itself takes care of all the required conditions,
and the "a > b ||" has no effect except to make auditors
scratch their heads.
Index: diffreg.c
===================================================================
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.91
diff -u -p -r1.91 diffreg.c
--- diffreg.c 1 Mar 2016 20:57:35 -0000 1.91
+++ diffreg.c 2 Apr 2018 18:57:02 -0000
@@ -972,21 +972,17 @@ restart:
* match an ignore pattern for the change to be
* ignored.
*/
- if (a <= b) { /* Changes and deletes. */
- for (i = a; i <= b; i++) {
- line = preadline(fileno(f1),
- ixold[i] - ixold[i - 1], ixold[i - 1]);
- if (!ignoreline(line))
- goto proceed;
- }
+ for (i = a; i <= b; i++) { /* Changes and deletes. */
+ line = preadline(fileno(f1),
+ ixold[i] - ixold[i - 1], ixold[i - 1]);
+ if (!ignoreline(line))
+ goto proceed;
}
- if (a > b || c <= d) { /* Changes and inserts. */
- for (i = c; i <= d; i++) {
- line = preadline(fileno(f2),
- ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
- if (!ignoreline(line))
- goto proceed;
- }
+ for (i = c; i <= d; i++) { /* Changes and inserts. */
+ line = preadline(fileno(f2),
+ ixnew[i] - ixnew[i - 1], ixnew[i - 1]);
+ if (!ignoreline(line))
+ goto proceed;
}
return;
}
