Re: [PATCH/RFC 2/5] compat/terminal: factor out echo-disabling

From: Jeff King <peff@peff.net>
Date: Nov 30, 2012, 18:19 UTC
Message-ID: <20121130181932.GB7197@sigill.intra.peff.net>
In-Reply-To: <alpine.DEB.1.00.1211301858570.31987@s15462909.onlinehome-server.info>

On Fri, Nov 30, 2012 at 06:59:30PM +0100, Johannes Schindelin wrote:

Show 13 quoted lines

> > diff --git a/compat/terminal.c b/compat/terminal.c
> > index bbb038d..3217838 100644
> > --- a/compat/terminal.c
> > +++ b/compat/terminal.c
> > @@ -14,6 +14,7 @@ static void restore_term(void)
> >  		return;
> >  
> >  	tcsetattr(term_fd, TCSAFLUSH, &old_term);
> > +	close(term_fd);
> >  	term_fd = -1;
> >  }
> 
> That looks like an independent resource leak fix... correct?

I don't think so. In the current code, term_fd does not take ownership of the fd. It is fundamentally part of the FILE* in git_terminal_prompt, and is closed when we fclose() that. But in Erik's refactor, we actually open a _second_ descriptor to /dev/tty, which needs to be closed.

I don't think there is any reason that should not work (the terminal characteristics should be per-tty, not per-descriptor), though it does feel a little hacky to have to open /dev/tty twice.

-Peff

-- 
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.

You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en