Thanks, this looks good. But I have my usual nitpicky comments
376 // sets the console echo on/off 377 private static native boolean echo(boolean on) throws IOException; I would document the return value. @returns the previous console echo on/off status 314 boolean echoWasOn = true; I much prefer leaving it blank (in Java, not C !) boolean echoWasOn; relying on definite assignment in Java for safety. On Mon, Apr 16, 2018 at 12:18 PM, Xueming Shen <xueming.s...@oracle.com> wrote: > On 4/13/18, 8:04 PM, Martin Buchholz wrote: > > This is trickier than I expected, since you have to manage > saving/restoring around each call to readPassword AND have an exit hook to > restore in case the user never gets around to responding to the prompt. > > I see in the old code echo returns a boolean that you would think could be > used to restore. You want to save/restore around each readPassword AND > around the entire java program. It still seems right to restore after > every readpassword, using the returned value from echo (which the suggested > fix ignores!). But also, the very first time readPassword is called, you > create an exit hook to restore the value known at that time. So you need > echo0 as well ... or do you ... maybe you just need to keep track of a > single state variable for you to do first-time setup. > > Updated to use a local flag for each "echo-off" setting for > readPassword(), so the > echo will not be turned on after readpw if its initial status is off > before readpw. > > Still keep the global one & exit-hook for use scenario that someone ctrl-D > to exit > or the vm gets -kill signal during passwd reading. > > webrev has been updated accordingly (with the updates based on other > comments). > > http://cr.openjdk.java.net/~sherman/8194750/webrev > > Thanks! > Sherman > > > + return tio.c_lflag & ECHO; > > Pedantically, functions returning jboolean should return only JNI_TRUE and > JNI_FALSE. I would do: > > return (tio.c_lflag & ECHO) != 0; > > although there's the even more pedantic > > return ((tio.c_lflag & ECHO) != 0) ? JNI_TRUE : JNI_FALSE; > > typo: chosole > > > > >
