Bug 140939

Summary:	--track-fds reports leakage of stdout/in/err and doesn't respect -q
Product:	[Developer tools] valgrind	Reporter:	Peter Kelly <pmk>
Component:	memcheck	Assignee:	Mark Wielaard <mark>
Status:	RESOLVED FIXED
Severity:	normal	CC:	alan.christopher.jenkins, daniel, david, mark, njn
Priority:	NOR
Version First Reported In:	unspecified
Target Milestone:	wanted3.6.0
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	patch against valgrind 3.2.3 to modify --track-fds output patch against valgrind 3.16-git to modify --track-fds output Patch including --track-fds=all, testsuite updates and documentation

Description Peter Kelly 2007-01-31 06:54:30 UTC

Version:           3.2.3 (using KDE Devel)
Installed from:    Compiled sources
Compiler:          gcc 3.4.6 
OS:                Linux

I'm using the --track-fds option to detect file descriptor leakage in my application. I've noticed that it includes file descriptors 0, 1, and 2 (stdin, stdout, and stderr) in its report. Ideally these should not be reported as errors since virtually all programs do not close these on exit. Having these open when the program exits is not really a bug; the main thing most people would want to know from --track-fds is if they have inadvertently left other files or sockets open.

Additionally, the -q option is supposed to cause valgrind to only print output on error conditions. Even with a program that does close stdin/out/err before exiting, the message "FILE DESCRIPTORS: 0 open at exit" is still reported. Ideally this message should only be printed if -q is not specified, and only file descriptors greater than 2 should be listed (and included in the count).

I have written a patch which modifies the function VG_(show_open_fds) to give this behaviour which I will try to attach to this bug report.

Comment 1 Peter Kelly 2007-01-31 06:55:20 UTC

Created attachment 19491 [details]
patch against valgrind 3.2.3 to modify --track-fds output

Comment 2 Julian Seward 2008-11-05 14:41:18 UTC

Seems reasonable.  Will commit unless anyone complains.

Comment 3 Alan Jenkins 2008-12-20 11:31:58 UTC

Has the patch been applied yet?

Comment 4 David van Laatum 2009-08-06 12:01:04 UTC

I think it really should check to see if those fds are a terminal as in some situations they may have been closed and replaced with others

Comment 5 David van Laatum 2009-08-06 12:18:18 UTC

also should verify that its the controlling terminal if possible but not sure
how or if this is possible

Comment 6 Daniel Fahlgren 2020-02-08 20:40:36 UTC

Created attachment 125774 [details]
patch against valgrind 3.16-git to modify --track-fds output

This updated patch is against the current code in git

Comment 7 Mark Wielaard 2021-02-07 23:40:13 UTC

Created attachment 135494 [details]
Patch including --track-fds=all, testsuite updates and documentation

For some reason this keeps being dropped while it is a good idea and people approve of the fix. I updated the patch a little to include a new variant --track-fds=all which does include tracking/reporting on stdin/out/err. It also updates the testsuite and documentation.

Comment 8 Mark Wielaard 2021-02-10 20:12:38 UTC

commit c9781cc97e719c1ecf0cfad9ca2f86631c017268
Author: Mark Wielaard <mark@klomp.org>
Date:   Mon Feb 8 00:25:52 2021 +0100

    PR140939 --track-fds reports leakage of stdout/in/err and doesn't respect -q
    
    Make --track-fds=yes not report on file descriptors 0, 1, and 2 (stdin,
    stdout, and stderr) by default. Add a new option --track-fds=all that does
    report on the std file descriptors still being open. Update testsuite and
    documentation.
    
    Original patch by Peter Kelly <pmk@cs.adelaide.edu.au>
    Updated by Daniel Fahlgren <daniel@fahlgren.se>
    
    https://bugs.kde.org/show_bug.cgi?id=140939