{"thread":{"id":"31192","subject":"[PATCH] Avoid crippled getpass function on Solaris","startedAt":"2012-08-05T23:17:52Z","lastAt":"2012-08-08T21:05:31Z","messageCount":27,"participants":["Ben Walton","Tay Ray Chuan","Junio C Hamano","Jeff King","Andreas Schwab","Erik Faye-Lund"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"196518","messageId":"1344208672-20028-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"31192","inReplyTo":null,"subject":"[PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-05T23:17:52Z","receivedAt":"2012-08-05T23:17:52Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"On Solaris getpass() returns at most 8 characters which cripples the\ncredential reading for accounts using longer passwords. The alternate\nfunction getpassphrase() was introduced in SunOS 5.6 and will return\nup to 256 characters.\n\nEnsure that git_terminal_prompt uses the more able function when\nbuilding on Solaris.\n\nSigned-off-by: Ben Walton \n---\n\nHi Jeff and Junio,\n\nI considered making this minor change a few different ways but settled\non this as it seemed (to my eye) to most closely adhere to the way\nother such things were done in the compatibility code. I'm entirely\nopen to modifying this if it's felt that there is a clearer/cleaner\nway to do it.\n\nI'd even considered making the function swap generic enough to be\ndriven by the build system. That seemed over the top though, given\nthat most systems either have a decent getpass() or don't use this\ncode path at all.\n\nI've also briefly dabbled with getting Solaris to simply use the\nHAVE_DEV_TTY code path but the terminal echo stuff hasn't worked\nnicely for me just yet. (It reads the password with nothing echoed\nbut then displays the string after reading the newline.) This might\nstill be a better approach in the future, but for now, having long\npassword reading capability will still be a benefit to users on this\nplatform.\n\nThanks\n-Ben\n\n compat/terminal.c | 2 +-\n compat/terminal.h | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 6d16c8f..e1ab536 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -75,7 +75,7 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \n char *git_terminal_prompt(const char *prompt, int echo)\n {\n-\treturn getpass(prompt);\n+\treturn GETPASS(prompt);\n }\n \n #endif\ndiff --git a/compat/terminal.h b/compat/terminal.h\nindex 97db7cd..8d7b3f9 100644\n--- a/compat/terminal.h\n+++ b/compat/terminal.h\n@@ -3,4 +3,13 @@\n \n char *git_terminal_prompt(const char *prompt, int echo);\n \n+/* getpass() returns at most 8 characters on solaris so use\n+ getpassphrase() which returns up to 256. */\n+# if defined (__SVR4) && defined (__sun) /* solaris */\n+#define GETPASS getpassphrase\n+#else\n+#define GETPASS getpass\n+#endif\n+\n+\n #endif /* COMPAT_TERMINAL_H */\n-- \n1.7.10.3\n"},{"id":"196528","messageId":"CALUzUxrfSEtkXfFbotb3XARt9e=oWF0xQRVU0hwnsCJn-Cn0Lw@mail.gmail.com","threadId":"31192","inReplyTo":"1344208672-20028-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-08-06T01:56:55Z","receivedAt":"2012-08-06T01:56:55Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Mon, Aug 6, 2012 at 7:17 AM, Ben Walton wrote:\n> I've also briefly dabbled with getting Solaris to simply use the\n> HAVE_DEV_TTY code path but the terminal echo stuff hasn't worked\n> nicely for me just yet. (It reads the password with nothing echoed\n> but then displays the string after reading the newline.) This might\n> still be a better approach in the future, but for now, having long\n> password reading capability will still be a benefit to users on this\n> platform.\n\nReplacing\n\n\tif (!echo) {\n\t\tputc('\\n', fh);\n\t\tfflush(fh);\n\t}\n\nwith\n\n\tif (!echo)\n\t\twrite(term_fd, \"\\n\", 1);\n\nfixed that. Using fd's instead of FILE* was mentioned at [1]. Perhaps\nthat is the direction to go in.\n\n[1] http://mid.gmane.org/<7vsjc12j5o.fsf@alter.siamese.dyndns.org>\n\n-- \nCheers,\nRay Chuan\n"},{"id":"196530","messageId":"7vboio231n.fsf@alter.siamese.dyndns.org","threadId":"31192","inReplyTo":"1344208672-20028-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-06T01:59:48Z","receivedAt":"2012-08-06T01:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton writes:\n\n> diff --git a/compat/terminal.h b/compat/terminal.h\n> index 97db7cd..8d7b3f9 100644\n> --- a/compat/terminal.h\n> +++ b/compat/terminal.h\n> @@ -3,4 +3,13 @@\n> \n> char *git_terminal_prompt(const char *prompt, int echo);\n> \n> +/* getpass() returns at most 8 characters on solaris so use\n> + getpassphrase() which returns up to 256. */\n> +# if defined (__SVR4) && defined (__sun) /* solaris */\n> +#define GETPASS getpassphrase\n> +#else\n> +#define GETPASS getpass\n> +#endif\n> +\n> +\n> #endif /* COMPAT_TERMINAL_H */\n\nWouldn't\n\n\t#if solaris\n #define getpass getpassphrase\n #endif\n\nwithout anything else be more than sufficient?\n"},{"id":"196531","messageId":"1344220427-sup-3468@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"7vboio231n.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T02:35:06Z","receivedAt":"2012-08-06T02:35:06Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012:\n> Wouldn't\n> \n> #if solaris\n> #define getpass getpassphrase\n> #endif\n> \n> without anything else be more than sufficient?\n\nYes, it would, but I was hoping to make it more explicit that the\nfunction getpass may be substituted with something else.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196532","messageId":"1344220896-sup-7172@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"CALUzUxrfSEtkXfFbotb3XARt9e=oWF0xQRVU0hwnsCJn-Cn0Lw@mail.gmail.com","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T02:41:39Z","receivedAt":"2012-08-06T02:41:39Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Tay Ray Chuan's message of Sun Aug 05 21:56:55 -0400 2012:\n\n> > I've also briefly dabbled with getting Solaris to simply use the\n> > HAVE_DEV_TTY code path but the terminal echo stuff hasn't worked\n> > nicely for me just yet. (It reads the password with nothing\n> > echoed but then displays the string after reading the newline.)\n> > This might still be a better approach in the future, but for now,\n> > having long password reading capability will still be a benefit to\n> > users on this platform.\n> \n> Replacing\n> \n> if (!echo) {\n> putc('\\n', fh);\n> fflush(fh);\n> }\n> \n> with\n> \n> if (!echo)\n> write(term_fd, \"\\n\", 1);\n> \n> fixed that. Using fd's instead of FILE* was mentioned at [1]. Perhaps\n> that is the direction to go in.\n\nOh, interesting. I'd missed your update of this thread earlier\ntoday. It may make sense to do everything via file descriptors\ndirectly as you suggested in your comments on that patch.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196555","messageId":"20120806193958.GA10039@sigill.intra.peff.net","threadId":"31192","inReplyTo":"1344220427-sup-3468@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-06T19:39:58Z","receivedAt":"2012-08-06T19:39:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 05, 2012 at 10:35:06PM -0400, Ben Walton wrote:\n\n> Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012:\n> > Wouldn't\n> > \n> > #if solaris\n> > #define getpass getpassphrase\n> > #endif\n> > \n> > without anything else be more than sufficient?\n> \n> Yes, it would, but I was hoping to make it more explicit that the\n> function getpass may be substituted with something else.\n\nI don't think that's important. Either the thing is a drop-in replica of\ngetpass, or it is not. In the former case, it's OK for it to be\ntransparent that it has been replaced. In the latter case, it should not\nbe a #define replacement at all, but should be its own alternative in\ncompat/terminal.c (just like HAVE_DEV_TTY is). From my reading of\ngetpassphrase, it does seem to be a drop-in replacement.\n\nSo I'm OK conceptually with the patch if we can't do any better. But\ngetpass still sucks. It doesn't handle echoing, and it may or may not\nfall back to reading from stdin if the tty isn't available (which is\ndisastrous for remote-curl, whose stdin is speaking the remote-helper\nprotocol to git). So I'd really prefer to make HAVE_DEV_TTY work with\nSolaris if we can.\n\nI'm happy to spend a few cycles on it. I don't have access to any real\nSolaris boxes these days, but I imagine I can get OpenSolaris running\nunder VirtualBox without too much trouble...\n\n-Peff\n\nPS If we do go the getpassphrase route, does it make sense to introduce\n HAVE_GETPASSPHRASE? We usually try to provide one layer of\n indirection by naming our #defines after features, and then\n connecting systems to the feature defines via the Makefile. But maybe\n Solaris is the only system that has getpassphrase.\n"},{"id":"196558","messageId":"7v628vztc4.fsf@alter.siamese.dyndns.org","threadId":"31192","inReplyTo":"20120806193958.GA10039@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-06T19:57:47Z","receivedAt":"2012-08-06T19:57:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> .... From my reading of\n> getpassphrase, it does seem to be a drop-in replacement.\n>\n> So I'm OK conceptually with the patch if we can't do any better. But\n> getpass still sucks. It doesn't handle echoing, and it may or may not\n> fall back to reading from stdin if the tty isn't available (which is\n> disastrous for remote-curl, whose stdin is speaking the remote-helper\n> protocol to git). So I'd really prefer to make HAVE_DEV_TTY work with\n> Solaris if we can.\n\nTay Ray Chuan mentioned in the thread near-by that HAVE_DEV_TTY\ncodepath worked for him with \"FILE *\" to fd conversion, I think.\n"},{"id":"196568","messageId":"1344287843-sup-6200@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120806193958.GA10039@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T21:31:00Z","receivedAt":"2012-08-06T21:31:00Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Mon Aug 06 15:39:58 -0400 2012:\n> On Sun, Aug 05, 2012 at 10:35:06PM -0400, Ben Walton wrote:\n> \n> > Excerpts from Junio C Hamano's message of Sun Aug 05 21:59:48 -0400 2012:\n> > > Wouldn't\n> > > \n> > > #if solaris\n> > > #define getpass getpassphrase\n> > > #endif\n> > > \n> > > without anything else be more than sufficient?\n> > \n> > Yes, it would, but I was hoping to make it more explicit that the\n> > function getpass may be substituted with something else.\n> \n> I don't think that's important. Either the thing is a drop-in replica of\n> getpass, or it is not. In the former case, it's OK for it to be\n> transparent that it has been replaced. In the latter case, it should not\n> be a #define replacement at all, but should be its own alternative in\n> compat/terminal.c (just like HAVE_DEV_TTY is). From my reading of\n> getpassphrase, it does seem to be a drop-in replacement.\n\nIt is a drop in replacement (identical signature) so that's no issue.\nI can simplify this to the form suggested by Junio if that's\npreferrable. My idea of explicitness was for the programmer though.\nI think it's easier for someone else reading this code later to\nrealize that the GETPASS function may change depending on conditions.\n\nBut, as I said, I'm not adverse to changing this in any way, I simply\nwant the improved functionality and am not married to the approach.\n(I considered about three different ways of doing this before\nsubmitting.)\n\n> So I'm OK conceptually with the patch if we can't do any better. But\n> getpass still sucks. It doesn't handle echoing, and it may or may\n> not fall back to reading from stdin if the tty isn't available\n> (which is disastrous for remote-curl, whose stdin is speaking the\n> remote-helper protocol to git). So I'd really prefer to make\n> HAVE_DEV_TTY work with Solaris if we can.\n\nI poked through getpass in the opensolaris code and it did fall back\nto reading from stdin[1] if there were issues opening /dev/tty so\nthere is room for trouble here. That doesn't mean that other versions\nof Solaris did it this way, but it's fairly likely that they did given\nthe culture.\n\n> I'm happy to spend a few cycles on it. I don't have access to any\n> real Solaris boxes these days, but I imagine I can get OpenSolaris\n> running under VirtualBox without too much trouble...\n\nI'm also happy to do this work and have ready access to Solaris 8-11.\nWould it be reasonable to support using getpassphrase() if\n!HAVE_DEV_TTY in addition to making the HAVE_DEV_TTY code function on\nSolaris too? That would provide a harm reduction for Solaris users\nthat (for some reason) disabled the nicer interface...but maybe it's\ntoo much?\n\n> PS If we do go the getpassphrase route, does it make sense to\n> introduce HAVE_GETPASSPHRASE? We usually try to provide one layer\n> of indirection by naming our #defines after features, and then\n> connecting systems to the feature defines via the Makefile. But\n> maybe Solaris is the only system that has getpassphrase.\n\nI had considered this but thought that it was too heavy weight given\nthat I'm not aware of other platforms that have this naming split\ndepending on desired buffer size and that getpass on most platforms\nwon't have this crippling size limitation.\n\nIf it's worth improving both paths for the HAVE_DEV_TTY code on\nSolaris, I can add this support if that's considered the better\napproach.\n\nThanks\n-Ben\n\n[1] http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/lib/libbc/libc/gen/common/getpass.c#65\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196569","messageId":"20120806213404.GA14320@sigill.intra.peff.net","threadId":"31192","inReplyTo":"1344287843-sup-6200@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-06T21:34:04Z","receivedAt":"2012-08-06T21:34:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 05:31:00PM -0400, Ben Walton wrote:\n\n> > I'm happy to spend a few cycles on it. I don't have access to any\n> > real Solaris boxes these days, but I imagine I can get OpenSolaris\n> > running under VirtualBox without too much trouble...\n> \n> I'm also happy to do this work and have ready access to Solaris 8-11.\n\nI'm currently in pkgutil hell trying to remember how to get a working\ngcc on Solaris. Bleh. What kind of OS doesn't ship with a compiler by\ndefault? :)\n\n> Would it be reasonable to support using getpassphrase() if\n> !HAVE_DEV_TTY in addition to making the HAVE_DEV_TTY code function on\n> Solaris too? That would provide a harm reduction for Solaris users\n> that (for some reason) disabled the nicer interface...but maybe it's\n> too much?\n\nIf you think there is a reason that somebody on Solaris might not want\nto use HAVE_DEV_TTY, then I guess it makes sense. But I can't really\nthink of one, so I'd be just as happy to leave it until somebody does.\n\n-Peff\n"},{"id":"196571","messageId":"1344290892-sup-1108@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120806213404.GA14320@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T22:09:08Z","receivedAt":"2012-08-06T22:09:08Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Mon Aug 06 17:34:04 -0400 2012:\n> On Mon, Aug 06, 2012 at 05:31:00PM -0400, Ben Walton wrote:\n> \n> > > I'm happy to spend a few cycles on it. I don't have access to any\n> > > real Solaris boxes these days, but I imagine I can get OpenSolaris\n> > > running under VirtualBox without too much trouble...\n> > \n> > I'm also happy to do this work and have ready access to Solaris 8-11.\n> \n> I'm currently in pkgutil hell trying to remember how to get a working\n> gcc on Solaris. Bleh. What kind of OS doesn't ship with a compiler by\n> default? :)\n\nOne that's losing mindshare and isn't gaining traction? *g* Feel free\nto message me offline if you need a hand with that.\n\n> > Would it be reasonable to support using getpassphrase() if\n> > !HAVE_DEV_TTY in addition to making the HAVE_DEV_TTY code function\n> > on Solaris too? That would provide a harm reduction for Solaris\n> > users that (for some reason) disabled the nicer interface...but\n> > maybe it's too much?\n> \n> If you think there is a reason that somebody on Solaris might not\n> want to use HAVE_DEV_TTY, then I guess it makes sense. But I can't\n> really think of one, so I'd be just as happy to leave it until\n> somebody does.\n\nNo, I can't think of a _good_ reason to do that either.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196572","messageId":"20120806223113.GA16298@sigill.intra.peff.net","threadId":"31192","inReplyTo":"1344290892-sup-1108@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-06T22:31:13Z","receivedAt":"2012-08-06T22:31:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 06:09:08PM -0400, Ben Walton wrote:\n\n> > I'm currently in pkgutil hell trying to remember how to get a working\n> > gcc on Solaris. Bleh. What kind of OS doesn't ship with a compiler by\n> > default? :)\n> \n> One that's losing mindshare and isn't gaining traction? *g* Feel free\n> to message me offline if you need a hand with that.\n\nThanks, I figured it out (I installed opencsw's gcc, but I had no\nstandard include files. Turns out that I needed the system/header\npackage from upstream).\n\nThe stdio behavior on Solaris is weird. If I run this sample program:\n\n #include \n int main(void)\n {\n FILE *fh = fopen(\"/dev/tty\", \"w+\");\n char buf[32] = {0};\n fgets(buf, sizeof(buf), fh);\n fprintf(fh, \"got %s\\n\", buf);\n return 0;\n }\n\non Linux, I get:\n\n $ ./a.out\n foo <-- me typing\n got foo <-- program output\n\nOn Solaris, I get:\n\n $ ./a.out\n foo <-- me typing\n foo <-- ???\n got foo <-- program output\n\nIf you step it through the debugger, the mystery output comes as part of\nthe fprintf, as if it is in the buffer waiting to be flushed. And\nindeed, if you dump the FILE handle via gdb, there is only a single\nbuffer, and it contains \"foo\\n\". Whereas with glibc, there are separate\nread/write buffers.\n\nI suspect the single buffer is enough to handle normal files, but not\nbidirectional pipes where the input and output content are unrelated. I\ndon't think Solaris is _buggy_ per se, as we have probably stepped\noutside the realm of what the standard promises, but it's certainly a\nquality-of-implementation issue.\n\nSo I think it could be solved by opening /dev/tty twice (or fdopen()ing\nthe same descriptor twice). Or by just doing away with stdio entirely.\n\nLooking over the code, I recall now why I used stdio: strbuf_getline\nrequires it. These days we have strbuf_getwholeline_fd. It's slightly\nless efficient (it has to read() a character at a time), but since we\nare talking about a few characters of keyboard input, it should be OK.\n\n-Peff\n"},{"id":"196573","messageId":"1344292395-sup-1921@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120806223113.GA16298@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T22:39:55Z","receivedAt":"2012-08-06T22:39:55Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Mon Aug 06 18:31:13 -0400 2012:\n\n> Looking over the code, I recall now why I used stdio: strbuf_getline\n> requires it. These days we have strbuf_getwholeline_fd. It's\n> slightly less efficient (it has to read() a character at a time),\n> but since we are talking about a few characters of keyboard input,\n> it should be OK.\n\nI noticed the same requirement. I'm currently building/testing a\npatch that switches from FILE -> fd and also to\nstrbuf_getwholeline_fd. Personally, I like this approach more than\ncalling an open function multiple times.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196574","messageId":"20120806224222.GA16721@sigill.intra.peff.net","threadId":"31192","inReplyTo":"1344292395-sup-1921@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-06T22:42:22Z","receivedAt":"2012-08-06T22:42:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 06:39:55PM -0400, Ben Walton wrote:\n\n> Excerpts from Jeff King's message of Mon Aug 06 18:31:13 -0400 2012:\n> \n> > Looking over the code, I recall now why I used stdio: strbuf_getline\n> > requires it. These days we have strbuf_getwholeline_fd. It's\n> > slightly less efficient (it has to read() a character at a time),\n> > but since we are talking about a few characters of keyboard input,\n> > it should be OK.\n> \n> I noticed the same requirement. I'm currently building/testing a\n> patch that switches from FILE -> fd and also to\n> strbuf_getwholeline_fd. Personally, I like this approach more than\n> calling an open function multiple times.\n\nOK, I'll stop working on the same thing, then. :)\n\nWe might want to do this to factor out the extra chomp you'll need by\nusing strbuf_getwholeline_fd:\n\n-- >8 --\nSubject: [PATCH] strbuf: implement strbuf_getline_fd\n\nWhen reading a line into a strbuf, we have two options:\n\n 1. Read from stdio, or from a descriptor\n\n 2. Chomp the terminator, or leave it in\n\nWe already have:\n\n strbuf_getwholeline: stdio, nochomp\n strbuf_getline: stdio, chomp\n strbuf_getwholeline_fd: fd, nochomp\n\nThis patch adds the missing orthogonal function:\n\n strbuf_getline_fd: fd, chomp\n---\n strbuf.c | 8 ++++++++\n strbuf.h | 1 +\n 2 files changed, 9 insertions(+)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex ec88266..a6dffcc 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -399,6 +399,14 @@ int strbuf_getwholeline_fd(struct strbuf *sb, int fd, int term)\n \treturn 0;\n }\n \n+int strbuf_getline_fd(struct strbuf *sb, int fd, int term)\n+{\n+\tif (strbuf_getwholeline(sb, fd, term))\n+\t\treturn EOF;\n+\tif (sb->buf[sb->len-1] == term)\n+\t\tstrbuf_setlen(sb, sb->len-1);\n+}\n+\n int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n {\n \tint fd, len;\ndiff --git a/strbuf.h b/strbuf.h\nindex b888d40..cc1abb2 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -119,6 +119,7 @@ extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_getwholeline(struct strbuf *, FILE *, int);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n extern int strbuf_getwholeline_fd(struct strbuf *, int, int);\n+extern int strbuf_getline_fd(struct strbuf *, int, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n"},{"id":"196575","messageId":"m2pq73zkmu.fsf@igel.home","threadId":"31192","inReplyTo":"20120806223113.GA16298@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-08-06T23:05:45Z","receivedAt":"2012-08-06T23:05:45Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Jeff King writes:\n\n> The stdio behavior on Solaris is weird. If I run this sample program:\n>\n> #include \n> int main(void)\n> {\n> FILE *fh = fopen(\"/dev/tty\", \"w+\");\n> char buf[32] = {0};\n> fgets(buf, sizeof(buf), fh);\n> fprintf(fh, \"got %s\\n\", buf);\n> return 0;\n> }\n>\n> on Linux, I get:\n>\n> $ ./a.out\n> foo <-- me typing\n> got foo <-- program output\n>\n> On Solaris, I get:\n>\n> $ ./a.out\n> foo <-- me typing\n> foo <-- ???\n> got foo <-- program output\n\nThat's not a bug, you need to flush or seek when you want to switch\nbetween read to write.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"196576","messageId":"1344295889-sup-7899@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120806224222.GA16721@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-06T23:31:30Z","receivedAt":"2012-08-06T23:31:30Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Mon Aug 06 18:42:22 -0400 2012:\n\n> + if (strbuf_getwholeline(sb, fd, term))\n\nShouldn't this be strbuf_getwholeline_fd though?\n\nOtherwise, I think this is a good addition.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196577","messageId":"20120807000158.GA17022@sigill.intra.peff.net","threadId":"31192","inReplyTo":"1344295889-sup-7899@pinkfloyd.chass.utoronto.ca","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T00:01:58Z","receivedAt":"2012-08-07T00:01:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 07:31:30PM -0400, Ben Walton wrote:\n\n> Excerpts from Jeff King's message of Mon Aug 06 18:42:22 -0400 2012:\n> \n> > + if (strbuf_getwholeline(sb, fd, term))\n> \n> Shouldn't this be strbuf_getwholeline_fd though?\n\nWhoops, yes. When I got your email, I had just finished the patch but\nhad not yet written the follow-on patch that would actually exercise it.\n\n-Peff\n"},{"id":"196578","messageId":"20120807002318.GA17498@sigill.intra.peff.net","threadId":"31192","inReplyTo":"m2pq73zkmu.fsf@igel.home","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T00:23:18Z","receivedAt":"2012-08-07T00:23:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 07, 2012 at 01:05:45AM +0200, Andreas Schwab wrote:\n\n> > The stdio behavior on Solaris is weird. If I run this sample program:\n> >\n> > #include \n> > int main(void)\n> > {\n> > FILE *fh = fopen(\"/dev/tty\", \"w+\");\n> > char buf[32] = {0};\n> > fgets(buf, sizeof(buf), fh);\n> > fprintf(fh, \"got %s\\n\", buf);\n> > return 0;\n> > }\n> >\n> > on Linux, I get:\n> >\n> > $ ./a.out\n> > foo <-- me typing\n> > got foo <-- program output\n> >\n> > On Solaris, I get:\n> >\n> > $ ./a.out\n> > foo <-- me typing\n> > foo <-- ???\n> > got foo <-- program output\n> \n> That's not a bug, you need to flush or seek when you want to switch\n> between read to write.\n\nThanks. Inserting an fflush() before the fprintf does fix it, but I\nthink that a flush is disallowed by the standard in this case. From C99,\n7.19.5.2 (fflush):\n\n If stream points to an output stream or an update stream in which the\n most recent operation was not input, the fflush function causes any\n unwritten data for that stream to be delivered to the host environment\n to be written to the file; otherwise, the behavior is undefined.\n\nAnd later, from 7.19.5.3 (fopen):\n\n When a file is opened with update mode ('+' as the second or third\n character in the above list of mode argument values), both input and\n output may be performed on the associated stream. However, output\n shall not be directly followed by input without an intervening call to\n the fflush function or to a file positioning function (fseek, fsetpos,\n or rewind), and input shall not be directly followed by output without\n an intervening call to a file positioning function, unless the input\n operation encounters end-of-file.\n\nI don't know if any implementation actually cares in practice, of\ncourse, but probably the sane thing would be to call\n\"fseek(fh, SEEK_CUR, 0)\" before the fprintf.\n\nThis is all moot if we end up ripping stdio out of this code for other\nreasons, but it does give us another option for a fix.\n\nThanks for the pointer.\n\n-Peff\n"},{"id":"196579","messageId":"20120807003541.GA18219@sigill.intra.peff.net","threadId":"31192","inReplyTo":"20120807002318.GA17498@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T00:35:41Z","receivedAt":"2012-08-07T00:35:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 08:23:18PM -0400, Jeff King wrote:\n\n> This is all moot if we end up ripping stdio out of this code for other\n> reasons, but it does give us another option for a fix.\n\nAnd here is what that patch would look like:\n\n-- >8 --\nSubject: [PATCH] terminal: seek when switching between reading and writing\n\nWhen a stdio stream is opened in update mode (e.g., \"w+\"),\nthe C standard forbids switching between reading or writing\nwithout an intervening positioning function. Many\nimplementations are lenient about this, but Solaris libc\nwill flush the recently-read contents to the output buffer.\nIn this instance, that meant writing the non-echoed password\nthat the user just typed to the terminal.\n\nFix it by inserting a no-op fseek between the read and\nwrite.\n\nThe opposite direction (writing immediately followed by\nreading) is also disallowed, but our fflush immediately\nafter printing the prompt is sufficient to satisfy the\nstandard.\n---\n compat/terminal.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 6d16c8f..bbb038d 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -59,6 +59,7 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \n \tr = strbuf_getline(&buf, fh, '\\n');\n \tif (!echo) {\n+\t\tfseek(fh, SEEK_CUR, 0);\n \t\tputc('\\n', fh);\n \t\tfflush(fh);\n \t}\n"},{"id":"196582","messageId":"CALUzUxoKHSw_8U0S_jmrhW3fWF0OPL39+4q7385-_34xq8MXqQ@mail.gmail.com","threadId":"31192","inReplyTo":"20120807003541.GA18219@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2012-08-07T02:18:37Z","receivedAt":"2012-08-07T02:18:37Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Tue, Aug 7, 2012 at 8:35 AM, Jeff King wrote:\n> Subject: [PATCH] terminal: seek when switching between reading and writing\n>\n> When a stdio stream is opened in update mode (e.g., \"w+\"),\n> the C standard forbids switching between reading or writing\n> without an intervening positioning function. Many\n> implementations are lenient about this, but Solaris libc\n> will flush the recently-read contents to the output buffer.\n> In this instance, that meant writing the non-echoed password\n> that the user just typed to the terminal.\n>\n> Fix it by inserting a no-op fseek between the read and\n> write.\n>\n> The opposite direction (writing immediately followed by\n> reading) is also disallowed, but our fflush immediately\n> after printing the prompt is sufficient to satisfy the\n> standard.\n> ---\n> compat/terminal.c | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/compat/terminal.c b/compat/terminal.c\n> index 6d16c8f..bbb038d 100644\n> --- a/compat/terminal.c\n> +++ b/compat/terminal.c\n> @@ -59,6 +59,7 @@ char *git_terminal_prompt(const char *prompt, int echo)\n>\n> r = strbuf_getline(&buf, fh, '\\n');\n> if (!echo) {\n> + fseek(fh, SEEK_CUR, 0);\n> putc('\\n', fh);\n> fflush(fh);\n> }\n\nThis works. Ben, does this work for you too?\n\n-- \nCheers,\nRay Chuan\n"},{"id":"196583","messageId":"1344308342-sup-4817@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120807003541.GA18219@sigill.intra.peff.net","subject":"Re: [PATCH] Avoid crippled getpass function on Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-07T03:01:55Z","receivedAt":"2012-08-07T03:01:55Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Mon Aug 06 20:35:41 -0400 2012:\n\n> ---\n> compat/terminal.c | 1 +\n> 1 file changed, 1 insertion(+)\n> \n> diff --git a/compat/terminal.c b/compat/terminal.c\n> index 6d16c8f..bbb038d 100644\n> --- a/compat/terminal.c\n> +++ b/compat/terminal.c\n> @@ -59,6 +59,7 @@ char *git_terminal_prompt(const char *prompt, int echo)\n> \n> r = strbuf_getline(&buf, fh, '\\n');\n> if (!echo) {\n> + fseek(fh, SEEK_CUR, 0);\n> putc('\\n', fh);\n> fflush(fh);\n> }\n\nAcked-by: Ben Walton \n\nThat looks good to me. I'm able to clone a password protected https\nrepository and the prompting works as I'd expect.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196584","messageId":"1344308862-24635-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"31192","inReplyTo":"20120807003541.GA18219@sigill.intra.peff.net","subject":"[PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-07T03:07:42Z","receivedAt":"2012-08-07T03:07:42Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Now that git_terminal_prompt can cleanly interact with /dev/tty on\nSolaris, enable HAVE_DEV_TTY so that this code path is used for\ncredential reading instead of relying on the crippled getpass().\n\nSigned-off-by: Ben Walton \n---\n\nThis is a follow up to Jeff's patch that fixes git_terminal_prompt on\nSolaris. I don't have 5.6 or 5.7 for testing but I believe this\nshould be valid for both of those releases as well.\n\n\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 15d1319..6b0c961 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1014,6 +1014,7 @@ ifeq ($(uname_S),SunOS)\n \tNO_REGEX = YesPlease\n \tNO_FNMATCH_CASEFOLD = YesPlease\n \tNO_MSGFMT_EXTENDED_OPTIONS = YesPlease\n+\tHAVE_DEV_TTY = YesPlease\n \tifeq ($(uname_R),5.6)\n \t\tSOCKLEN_T = int\n \t\tNO_HSTRERROR = YesPlease\n-- \n1.7.10.3\n"},{"id":"196586","messageId":"7v4nofxt89.fsf@alter.siamese.dyndns.org","threadId":"31192","inReplyTo":"1344308862-24635-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-07T03:43:02Z","receivedAt":"2012-08-07T03:43:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton writes:\n\n> Now that git_terminal_prompt can cleanly interact with /dev/tty on\n> Solaris, enable HAVE_DEV_TTY so that this code path is used for\n> credential reading instead of relying on the crippled getpass().\n>\n> Signed-off-by: Ben Walton \n> ---\n>\n> This is a follow up to Jeff's patch that fixes git_terminal_prompt on\n> Solaris. I don't have 5.6 or 5.7 for testing but I believe this\n> should be valid for both of those releases as well.\n\nSo which direction do you guys want to go? Use the \"bidirectional\nstdio with fseek()\" for now, with the expectation that Tay's other\nseries will rewrite it to fd based one?\n\n>\n> Makefile | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/Makefile b/Makefile\n> index 15d1319..6b0c961 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1014,6 +1014,7 @@ ifeq ($(uname_S),SunOS)\n> \tNO_REGEX = YesPlease\n> \tNO_FNMATCH_CASEFOLD = YesPlease\n> \tNO_MSGFMT_EXTENDED_OPTIONS = YesPlease\n> +\tHAVE_DEV_TTY = YesPlease\n> \tifeq ($(uname_R),5.6)\n> \t\tSOCKLEN_T = int\n> \t\tNO_HSTRERROR = YesPlease\n"},{"id":"196587","messageId":"20120807040326.GA18682@sigill.intra.peff.net","threadId":"31192","inReplyTo":"7v4nofxt89.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T04:03:26Z","receivedAt":"2012-08-07T04:03:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 06, 2012 at 08:43:02PM -0700, Junio C Hamano wrote:\n\n> Ben Walton writes:\n> \n> > Now that git_terminal_prompt can cleanly interact with /dev/tty on\n> > Solaris, enable HAVE_DEV_TTY so that this code path is used for\n> > credential reading instead of relying on the crippled getpass().\n> >\n> > Signed-off-by: Ben Walton \n> > ---\n> >\n> > This is a follow up to Jeff's patch that fixes git_terminal_prompt on\n> > Solaris. I don't have 5.6 or 5.7 for testing but I believe this\n> > should be valid for both of those releases as well.\n> \n> So which direction do you guys want to go? Use the \"bidirectional\n> stdio with fseek()\" for now, with the expectation that Tay's other\n> series will rewrite it to fd based one?\n\nI think so. The stdio fix is short and obviously correct, and then Tay\ncan either refactor or not as he sees fit for his topic (although if we\ndo switch to just a terminal_can_prompt() interface and get rid of the\nterm_t ugliness, then there is not even any need to do the rewrite).\n\n-Peff\n"},{"id":"196588","messageId":"20120807041026.GA21918@sigill.intra.peff.net","threadId":"31192","inReplyTo":"20120807040326.GA18682@sigill.intra.peff.net","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-07T04:10:26Z","receivedAt":"2012-08-07T04:10:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 07, 2012 at 12:03:26AM -0400, Jeff King wrote:\n\n> > So which direction do you guys want to go? Use the \"bidirectional\n> > stdio with fseek()\" for now, with the expectation that Tay's other\n> > series will rewrite it to fd based one?\n> \n> I think so. The stdio fix is short and obviously correct, and then Tay\n> can either refactor or not as he sees fit for his topic (although if we\n> do switch to just a terminal_can_prompt() interface and get rid of the\n> term_t ugliness, then there is not even any need to do the rewrite).\n\nAnd here it is again, this time with a signed-off-by (I fixed my script\nafter our last discussion, but accidentally copied an old version to the\nSolaris VM I just installed. ;) ).\n\n-- >8 --\nSubject: [PATCH] terminal: seek when switching between reading and writing\n\nWhen a stdio stream is opened in update mode (e.g., \"w+\"),\nthe C standard forbids switching between reading or writing\nwithout an intervening positioning function. Many\nimplementations are lenient about this, but Solaris libc\nwill flush the recently-read contents to the output buffer.\nIn this instance, that meant writing the non-echoed password\nthat the user just typed to the terminal.\n\nFix it by inserting a no-op fseek between the read and\nwrite.\n\nThe opposite direction (writing followed by reading) is also\ndisallowed, but our intervening fflush is an acceptable\npositioning function for that alternative.\n\nSigned-off-by: Jeff King \n---\n compat/terminal.c | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/compat/terminal.c b/compat/terminal.c\nindex 6d16c8f..bbb038d 100644\n--- a/compat/terminal.c\n+++ b/compat/terminal.c\n@@ -59,6 +59,7 @@ char *git_terminal_prompt(const char *prompt, int echo)\n \n \tr = strbuf_getline(&buf, fh, '\\n');\n \tif (!echo) {\n+\t\tfseek(fh, SEEK_CUR, 0);\n \t\tputc('\\n', fh);\n \t\tfflush(fh);\n \t}\n"},{"id":"196605","messageId":"1344353369-sup-5103@pinkfloyd.chass.utoronto.ca","threadId":"31192","inReplyTo":"20120807041026.GA21918@sigill.intra.peff.net","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2012-08-07T15:31:48Z","receivedAt":"2012-08-07T15:31:48Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jeff King's message of Tue Aug 07 00:10:26 -0400 2012:\n\n> Signed-off-by: Jeff King \n\nAcked-by: Ben Walton \n\nI agree with your assesment that this is the right way to go (vs\nmigrating out of stdio) for now. If the changes Tay needs to make\nrequire the migration then it can become part of that series.\nOtherwise those changes would just be change for change's sake at this\ntime.\n\nIf my HAVE_DEV_TTY enabling patch for Solaris is added on top of this,\nI'm a happy camper.\n\nThanks\n-Ben\n--\nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n"},{"id":"196668","messageId":"CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com","threadId":"31192","inReplyTo":"20120807041026.GA21918@sigill.intra.peff.net","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2012-08-08T14:13:03Z","receivedAt":"2012-08-08T14:13:03Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Aug 7, 2012 at 6:10 AM, Jeff King wrote:\n> Subject: [PATCH] terminal: seek when switching between reading and writing\n>\n> When a stdio stream is opened in update mode (e.g., \"w+\"),\n> the C standard forbids switching between reading or writing\n> without an intervening positioning function. Many\n> implementations are lenient about this, but Solaris libc\n> will flush the recently-read contents to the output buffer.\n> In this instance, that meant writing the non-echoed password\n> that the user just typed to the terminal.\n>\n> Fix it by inserting a no-op fseek between the read and\n> write.\n\nMy Windows-patches for git_terminal_prompt would probably also solve\nthis problem. Instead of opening a read-write handle to /dev/tty, they\nopen two handles to the terminal instead; one for reading and one for\nwriting. This is because the terminal cannot be opened in read-write\nmode on Windows (we need to open \"CONIN$\" and \"CONOUT$\" separately).\n\nYou can have a look at the series here if you're interested:\nhttps://github.com/kusma/git/tree/work/terminal-cleanup\n\nThat last patch is the reason why I haven't submitted the series yet,\nbut perhaps some of the preparatory patches could be worth-while for\nother platforms in the mean time?\n"},{"id":"196707","messageId":"20120808210531.GA12822@sigill.intra.peff.net","threadId":"31192","inReplyTo":"CABPQNSaUCEDU4+2N63n0k_XwSXOP_iFZG3GEYSPSBPcSVV8wRQ@mail.gmail.com","subject":"Re: [PATCH] Enable HAVE_DEV_TTY for Solaris","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-08-08T21:05:31Z","receivedAt":"2012-08-08T21:05:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 08, 2012 at 04:13:03PM +0200, Erik Faye-Lund wrote:\n\n> On Tue, Aug 7, 2012 at 6:10 AM, Jeff King wrote:\n> > Subject: [PATCH] terminal: seek when switching between reading and writing\n> >\n> > When a stdio stream is opened in update mode (e.g., \"w+\"),\n> > the C standard forbids switching between reading or writing\n> > without an intervening positioning function. Many\n> > implementations are lenient about this, but Solaris libc\n> > will flush the recently-read contents to the output buffer.\n> > In this instance, that meant writing the non-echoed password\n> > that the user just typed to the terminal.\n> >\n> > Fix it by inserting a no-op fseek between the read and\n> > write.\n> \n> My Windows-patches for git_terminal_prompt would probably also solve\n> this problem. Instead of opening a read-write handle to /dev/tty, they\n> open two handles to the terminal instead; one for reading and one for\n> writing. This is because the terminal cannot be opened in read-write\n> mode on Windows (we need to open \"CONIN$\" and \"CONOUT$\" separately).\n\nYeah, it would solve it, although it means opening /dev/tty twice (which\nis probably not a big deal, though). I'm fine if we go that way in the\nlong run to share implementations, but let's treat it as a separate\ntopic. This fix is an obvious one-liner, and is just fixing me being\nstupid about actually following the C standard. So it's a no-brainer for\nas a maintenance fix.\n\n> You can have a look at the series here if you're interested:\n> https://github.com/kusma/git/tree/work/terminal-cleanup\n> \n> That last patch is the reason why I haven't submitted the series yet,\n> but perhaps some of the preparatory patches could be worth-while for\n> other platforms in the mean time?\n\nYeah, that last patch is really gross. There's no explanation of the\nrace issue, so I'll refrain from thinking about it until you are ready\nto post a series. :)\n\n-Peff\n"}]}