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
headius · Mar 2, 2021 · 5cbea1a · 5cbea1a
1 parent 9987e64
commit 5cbea1a
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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");