Re: [cpp patch]: Stop leaking file descriptors

["Zack Weinberg" <zackw at stanford dot edu>]

On Thu, Nov 02, 2000 at 01:00:03PM +0000, Nathan Sidwell wrote:
> Hi,
> cpp will leak file descriptors when reincluding an idempotent header file.
> What happens is that open_file (cppfiles.c) will reopen the header,
> but stack_unclude_file will simply ignore it, forgetting to close the
> descriptor. With something like gtk, which reincludes a lot of
> such headers, you run out of file descriptors!

Ouch.

> the patch stops open_file re-opening a DO_NOT_REREAD header -- was
> there a reason it should really be re-opened? 

No, it shouldn't.  This looks like a simple oversight to me.  The
whole point of the MI optimization is we open and read a file only
once.

> And makes sure that stack_include_file releases the file descriptor,
> if there was one.

The change to open_file is the proper fix for the bug, but the change to
stack_include_file should be unnecessary.  The file never stays open
past the end of read_include_file, or that's the way it's supposed to
work.

Could you please modify stack_include_file thus:

  int r;

  ...

  if (DO_NOT_REREAD (inc))
    {
      if (inc->fd != -1) abort ();
      return 0;
    }

  if (inc->buffer == NULL)
    if (read_include_file (pfile, inc) == 0)
      {
	if (inc->fd != -1) abort ();
        return 0;
      }

  if (inc->fd != -1) abort ();

and if any of those aborts ever happens, then find out why.  (Do this
with the open_file fix applied.)

If you get no aborts in a full bootstrap and testsuite run, then go
ahead and check in the change to open_file only.

> the alternative is that when open_file gets a EMFILE, it should walk
> the #include tree closing any DO_NOT_REREAD descriptors.

That's not nearly as reliable as closing the file when we're done with
it, and never opening it again, which is what's supposed to happen.

zw