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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi kusma,

On Sat, 1 Dec 2012, Erik Faye-Lund wrote:

> On Fri, Nov 30, 2012 at 6:59 PM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> 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@xxxxxxxxx>
> >> ---
> >>  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.

Thanks for clarifying,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]