On Fri, Sep 02, 2005 at 03:26:27AM +0100, Ian Lynagh wrote: > On Mon, Aug 29, 2005 at 07:53:21AM -0400, David Roundy wrote: > > > > hunk ./FastPackedString.hs 849 > > $ \p -> hGetBuf h p blocksize > > case lread of > > 0 -> return [] > > + l | l < blocksize -> do hClose h > > + return [PS fp 0 l] > > l -> do rest <- unsafeInterleaveIO read_rest > > return (PS fp 0 l:rest) > > unsafeInterleaveIO read_rest > > While this looks correct, I think it will mean that some bugs are harder > to find, as you need to have (>1000) files that are larger than the > buffer size, or perhaps are a multiple of the buffer size, to trigger > them. > > I don't think it'll actually solve any bugs, just some instances of > them.
I guess it depends on what you call bugs. I agree that it won't solve any higher-level leaks. But without this change, every time this function is called on a small file you're holding an open file descriptor for longer than is necesary, and that itself seems like a bug to me. When we know we've reached the end of a file, but fail to close it, that just seems wrong. We can always decrease the blocksize to trigger more bugs. In fact, perhaps we should add a "--enable-leak-testing" flag to the configure script, which could decrease both the blocksize and the mmap_limit, so that we could more easily trigger resource-exhaustion bugs. If we did that, we could make the test (perhaps just on darcs.net) configure with this flag, or perhaps we could make it run twice, once with and once without the flag (which might be overkill, but you're talking with someone whose work program, DFT++, runs its test suite five times with different configure options and at one point took over four hours). -- David Roundy http://www.darcs.net _______________________________________________ darcs-devel mailing list [email protected] http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
