The leak reported by Igor is in opentemp().
The only place opentemp() is called from is diffreg().
When diffreg() is called from main(), the only calls that
follow are print_status() and exit(), so we do not really
leak anything: opentemp() "leaks" ifd, returns NULL,
diffreg() returns, main() calls print_status() and exit().
>From diffdir(), diffreg() is only called for regular files,
and in that case, it never calls opentemp(), so the "leak"
cannot happen with diff -r.
There is no other place diffreg() is called from.
I doubt that we may ever want to change the above. Recursively
diffing directories containing special files by first copying
them to temp files using the read(2)-write(2)-loop in opentemp()
does not look like it might ever become a sane idea.
So i conclude the code is correct, and can't see how it risks
to become incorrect by changing it, unless the changes are
insane for unrelated reasons.
This analysis is a bit involved, so i guess we want to commit
the fix anyway? Just to make correctness of the code more obvious.
As whitespace in the original diff was mangled - two blanks
before leading tabs - here is a version that applies.
The following exercises the code path in question,
showing that both versions work as expected:
schwa...@rhea $ export TMPDIR=/nonexistent
schwa...@rhea $ diff /dev/null /dev/null
diff: /dev/null: No such file or directory
schwa...@rhea $ echo $?
2
schwa...@rhea $ ./obj/diff /dev/null /dev/null
diff: /dev/null: No such file or directory
schwa...@rhea $ echo $?
2
By the way, "/dev/null: No such file or directory"
looks funny, what actually fails is
mkstemp("/nonexistent/diff.XXXXXXXX")
of course.
Index: diffreg.c
===================================================================
RCS file: /cvs/src/usr.bin/diff/diffreg.c,v
retrieving revision 1.73
diff -u -p -r1.73 diffreg.c
--- diffreg.c 27 Oct 2009 23:59:37 -0000 1.73
+++ diffreg.c 22 Mar 2010 01:21:16 -0000
@@ -514,8 +514,10 @@ opentemp(const char *file)
return (NULL);
}
- if ((ofd = mkstemp(tempfile)) < 0)
+ if ((ofd = mkstemp(tempfile)) < 0) {
+ close(ifd);
return (NULL);
+ }
unlink(tempfile);
while ((nread = read(ifd, buf, BUFSIZ)) > 0) {
if (write(ofd, buf, nread) != nread) {
