Re: [PATCH] Enable HAVE_DEV_TTY for Solaris

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 [off-list ref] wrote:

quoted

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