From 5cbea1a326261377ba17f0e0a58414a0f87d190b Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Tue, 2 Mar 2021 15:11:26 -0600 Subject: [PATCH] Address review comments * Factor out real IO calc in IOOutputStream constructor * Revert to RubyThread.getStatus to preserve native status * Clean up some C-style declarations --- core/src/main/java/org/jruby/RubyIO.java | 14 ++++----- core/src/main/java/org/jruby/RubyThread.java | 4 +-- .../java/org/jruby/util/IOOutputStream.java | 30 +++++++++++-------- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyIO.java b/core/src/main/java/org/jruby/RubyIO.java index 3dbec5f4cbc..285b4f03317 100644 --- a/core/src/main/java/org/jruby/RubyIO.java +++ b/core/src/main/java/org/jruby/RubyIO.java @@ -1548,26 +1548,24 @@ public int write(ThreadContext context, byte[] bytes, int start, int length, En * @return the count of bytes written */ public int write(ThreadContext context, byte[] bytes, int start, int length, Encoding encoding, boolean nosync) { - Ruby runtime = context.runtime; - OpenFile fptr; - int n; - if (length == 0) return 0; - fptr = getOpenFileChecked(); + OpenFile fptr = getOpenFileChecked(); boolean locked = fptr.lock(); try { fptr = getOpenFileChecked(); fptr.checkWritable(context); - n = fptr.fwrite(context, bytes, start, length, encoding, nosync); - if (n == -1) throw runtime.newErrnoFromErrno(fptr.errno(), fptr.getPath()); + int n = fptr.fwrite(context, bytes, start, length, encoding, nosync); + + if (n == -1) throw context.runtime.newErrnoFromErrno(fptr.errno(), fptr.getPath()); + + return n; } finally { if (locked) fptr.unlock(); } - return n; } /** diff --git a/core/src/main/java/org/jruby/RubyThread.java b/core/src/main/java/org/jruby/RubyThread.java index fb1634191d1..c1a7c2af7b5 100644 --- a/core/src/main/java/org/jruby/RubyThread.java +++ b/core/src/main/java/org/jruby/RubyThread.java @@ -1255,7 +1255,7 @@ public RubyString inspect(ThreadContext context) { result.catString(Integer.toString(line + 1)); } result.cat(' '); - result.catString(status.toString().toLowerCase()); + result.catString(getStatus().toString().toLowerCase()); result.cat('>'); return result; } @@ -1673,7 +1673,7 @@ public void enterSleep() { } public void exitSleep() { - if (status != Status.ABORTING) { + if (getStatus() != Status.ABORTING) { STATUS.set(this, Status.RUN); } } diff --git a/core/src/main/java/org/jruby/util/IOOutputStream.java b/core/src/main/java/org/jruby/util/IOOutputStream.java index 15d8d9ebfb1..34d03bcead3 100644 --- a/core/src/main/java/org/jruby/util/IOOutputStream.java +++ b/core/src/main/java/org/jruby/util/IOOutputStream.java @@ -69,19 +69,7 @@ public IOOutputStream(final IRubyObject io, Encoding encoding, boolean checkAppe this.runtime = io.getRuntime(); // If we can get a real IO from the object, we can do fast writes - RubyIO realIO = null; - if (io instanceof RubyIO) { - RubyIO tmpIO = (RubyIO) io; - if (fastWritable(tmpIO)) { - tmpIO = tmpIO.GetWriteIO(); - - // recheck write IO for IOness - if (tmpIO == io || fastWritable(tmpIO)) { - realIO = tmpIO; - } - } - } - this.realIO = realIO; + RubyIO realIO = this.realIO = getRealIO(io); if (realIO == null || verifyCanWrite) { final String site; @@ -103,6 +91,22 @@ public IOOutputStream(final IRubyObject io, Encoding encoding, boolean checkAppe this.encoding = encoding; } + protected RubyIO getRealIO(IRubyObject io) { + RubyIO realIO = null; + if (io instanceof RubyIO) { + RubyIO tmpIO = (RubyIO) io; + if (fastWritable(tmpIO)) { + tmpIO = tmpIO.GetWriteIO(); + + // recheck write IO for IOness + if (tmpIO == io || fastWritable(tmpIO)) { + realIO = tmpIO; + } + } + } + return realIO; + } + protected boolean fastWritable(RubyIO io) { return !io.isClosed() && io.isBuiltin("write");