RFR: JDK-8194750, Console.readPassword does not save/restore tty settings

Martin Buchholz martinrb at google.com
Tue Apr 17 02:20:01 UTC 2018

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.shen at oracle.com>

> 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

More information about the core-libs-dev mailing list