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

> On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Tue, 13 Nov 2012, Erik Faye-Lund wrote:
> >
> >> By moving the echo-disabling code to a separate function, we can
> >> implement OS-specific versions of it for non-POSIX platforms.
> >>
> >> Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
> >> ---
> >>  compat/terminal.c | 43 +++++++++++++++++++++++++------------------
> >>  1 file changed, 25 insertions(+), 18 deletions(-)
> >>
> >> 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?
> 
> It might look like it, but it's not; term_fd used to be returned by
> "fileno(fh)", and fh did get properly closed.
> 
> With my refactoring, disable_echo/restore_term takes opens /dev/tty a
> second time, like Jeff points out. And that second file descriptor
> needs to be closed.