Re: [PATCH] Enable HAVE_DEV_TTY for Solaris

[Jeff King <peff@peff.net>]

On Wed, Aug 08, 2012 at 04:13:03PM +0200, Erik Faye-Lund wrote:

> On Tue, Aug 7, 2012 at 6:10 AM, Jeff King <peff@peff.net> wrote:
> > Subject: [PATCH] terminal: seek when switching between reading and writing
> >
> > When a stdio stream is opened in update mode (e.g., "w+"),
> > the C standard forbids switching between reading or writing
> > without an intervening positioning function. Many
> > implementations are lenient about this, but Solaris libc
> > will flush the recently-read contents to the output buffer.
> > In this instance, that meant writing the non-echoed password
> > that the user just typed to the terminal.
> >
> > Fix it by inserting a no-op fseek between the read and
> > write.
> 
> My Windows-patches for git_terminal_prompt would probably also solve
> this problem. Instead of opening a read-write handle to /dev/tty, they
> open two handles to the terminal instead; one for reading and one for
> writing. This is because the terminal cannot be opened in read-write
> mode on Windows (we need to open "CONIN$" and "CONOUT$" separately).

Yeah, it would solve it, although it means opening /dev/tty twice (which
is probably not a big deal, though). I'm fine if we go that way in the
long run to share implementations, but let's treat it as a separate
topic.  This fix is an obvious one-liner, and is just fixing me being
stupid about actually following the C standard. So it's a no-brainer for
as a maintenance fix.

> You can have a look at the series here if you're interested:
> https://github.com/kusma/git/tree/work/terminal-cleanup
> 
> That last patch is the reason why I haven't submitted the series yet,
> but perhaps some of the preparatory patches could be worth-while for
> other platforms in the mean time?

Yeah, that last patch is really gross. There's no explanation of the
race issue, so I'll refrain from thinking about it until you are ready
to post a series. :)

-Peff