RFR: 8224184: jshell got IOException at exiting with AIX

[Jan Lahoda]

Hi Toshio,

I think the change is fine, and I can sponsor the change.

It might be possible to create a unit-like test by doing something like:
class TestExecPty extends ExecPty {
     public List<String> getFlagsToSet(Attributes attr, Attributes 
current) {
         return super.getFlagsToSet(attr, current);
     }
}

and then:
new TestExecPty().getFlagsToSet(..., ...)

with carefully prepared Attributes sent to the method. (Having this 
would help to ensure this does not break on updates of the library.)

Thanks,
     Jan

On 11. 06. 19 15:09, Toshio 5 Nakamura wrote:
> Hi Robert,
> Thank you for review.
> 
> Hi Jan,
> Sorry for bothering you, but can we have your opinion by RDP1?
> 
> Thanks,
> Toshio Nakamura
> 
> Robert Field <robert.field at oracle.com> wrote on 2019/06/08 02:05:34:
> 
>  > From: Robert Field <robert.field at oracle.com>
>  > To: Toshio 5 Nakamura <TOSHIONA at jp.ibm.com>, kulla-
>  > dev at openjdk.java.net, Jan Lahoda <jan.lahoda at oracle.com>
>  > Date: 2019/06/08 02:05
>  > Subject: [EXTERNAL] Re: RFR: 8224184: jshell got IOException at
>  > exiting with AIX
>  >
>  > There are fixes for which the test is not possible or very hard, that is
>  > OK (there is even a special label for this case), they should still be
>  > fixed.
>  >
>  > The fix looks very reasonable to me.  You are checking for a failure
>  > case that wasn't checked for before.  The behavior (by code inspection,
>  > and your report) without your fix is clearly wrong.  Ignoring the
>  > character seems the best and safest choice.
>  >
>  > Jan is in charge of the internal JLine version.  I know he has a lot of
>  > pans in the fire right now.
>  >
>  > Jan: please let us know if I should be the sponsor of the patch, or you
>  > would?
>  >
>  > Thanks,
>  > Robert
>  >
>  >
>  > On 5/23/19 11:47 PM, Toshio 5 Nakamura wrote:
>  > > Hi,
>  > >
>  > > I think lacking of testcase may be a concern of my proposal.
>  > > Honestly, I couldn't create an automated testcase of this problem.
>  > > I tried to use shell script like below, but it cannot execute the
>  > > problematic path.
>  > >
>  > > $ echo "/exit" | jshell
>  > >
>  > > I think the testcase needs to run on 'pty'.
>  > >
>  > > Manual instructions are very simple, though.
>  > > 1. Open a terminal on AIX (or telnet/ssh to AIX)
>  > > 2. Launch jshell
>  > > 3. Type "/exit"
>  > >
>  > > Could you give me any advice?
>  > >
>  > > Issue: https://bugs.openjdk.java.net/browse/JDK-8224184
>  > > Webrev: http://cr.openjdk.java.net/~tnakamura/8224184/webrev.00/
>  > >
>  > > Thanks,
>  > > Toshio Nakamura
>  > > IBM Japan
>  > >
>  > >> From: "Toshio 5 Nakamura" <TOSHIONA at jp.ibm.com>
>  > >> To: kulla-dev at openjdk.java.net
>  > >> Date: 2019/05/20 17:13
>  > >> Subject: [EXTERNAL] RFR: 8224184: jshell got IOException at 
> exiting with
>  > > AIX
>  > >> Sent by: "kulla-dev" <kulla-dev-bounces at openjdk.java.net>
>  > >>
>  > >>
>  > >> Hi,
>  > >>
>  > >> Could you review JDK-8224184: jshell got IOException at exiting 
> with AIX?
>  > >> Also, I'd like to have a sponsor of the patch (I'm an author).
>  > >>
>  > >> Issue: INVALID URI REMOVED
>  > >>
>  > >
>  > 
> u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8224184&d=DwIFAg&c=jf_iaSHvJObTbx-
>  > >
>  > >> siA1ZOg&r=EVbFABcgo-X99_TGI2-
>  > >>
>  > >
>  > 
> qsMtyulHUruf8lAzMlVpVRqw&m=t8kIXQoVdkkBGhnWC09eRlCUxlXUlvabknJjA5SSotI&s=9LIfd1v5UBKEvYKAIm6tDtqYTlD6QK-
>  > >
>  > >> jRl1I8HHIu5g&e=
>  > >> Webrev: INVALID URI REMOVED
>  > >> u=http-3A__cr.openjdk.java.net_-7Etnakamura_8224184_webrev.
>  > >> 00&d=DwIFAg&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-
>  > >>
>  > >
>  > 
> qsMtyulHUruf8lAzMlVpVRqw&m=t8kIXQoVdkkBGhnWC09eRlCUxlXUlvabknJjA5SSotI&s=VC54L6HMLzUBB-3DcRQdr55JQda5wgw8r9SgxOvjkvA&e=
>  > >
>  > >> AIX doesn't show "min" or "time" by "stty -a" when "icanon" is 
> defined.
>  > >> This caused invalid value was trying to be set, and IOException was
>  > > thrown.
>  > >> Thanks,
>  > >> Toshio Nakamura
>  > >>
>  >
>