RFR 9: 8067796: (process) Process.waitFor(timeout, unit) doesn't throw NPE if timeout is less than, or equal to zero when unit == null

[Roger Riggs]

Hi Martin,

Thanks, I'll move the line up to be consistent with the local style.

Roger


On 3/20/2015 6:23 PM, Martin Buchholz wrote:
> I probably implemented it the way it is intentionally.  I would leave it.
>
> Pedantic changes such as this one won't make any users happy, but 
> we're doing this elsewhere like in java.util.concurrent so I guess 
> it's OK.
>
> But there's a much simpler way to implement it.
>
> Just move
>
> long remainingNanos = unit.toNanos(timeout);
> to the beginning of the method body, as we do in java.util.concurrent.
>
> On Fri, Mar 20, 2015 at 2:48 PM, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto:Roger.Riggs at oracle.com>> wrote:
>
>     Please review this change to Process.waitFor(timeout, TimeUnit)
>     to check for TimeUnit == null before checking for exited = true
>     and timeout <= 0.
>     The current implementation does not throw NPE if the timeout is <=
>     0 or the Process has already
>     exited.  The check for the validity of the arguments should come
>     before the other conditions.
>     This cleanup of the implementation was requested by the JCK team.
>
>     Webrev:
>          ://cr.openjdk.java.net/~rriggs/webrev-npe-8067796/
>     <http://cr.openjdk.java.net/%7Erriggs/webrev-npe-8067796/>
>     Issue:
>        8067796: (process) Process.waitFor(timeout, unit) doesn't throw
>     NPE if timeout is less than, or equal to zero when unit == null
>
>     Thanks, Roger
>
>     [1] https://bugs.openjdk.java.net/browse/JDK-8067796
>
>
>