Let IO#sync only affect writes, introduce IO#read_buffering? and us…

…e it in urandom (#6304)
crystal-lang · Jul 1, 2018 · 75ba915 · 75ba915
1 parent d6ba746
commit 75ba915
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 18 deletions.
diff --git a/spec/std/io/buffered_spec.cr b/spec/std/io/buffered_spec.cr
@@ -299,15 +299,17 @@ describe "IO::Buffered" do
       str.rewind
       str.read_byte.should eq(1_u8)
     end
+  end
 
+  describe "read_buffering" do
     it "works with IO#read" do
       str = IO::Memory.new "abc"
 
       io = BufferedWrapper.new(str)
-      io.sync?.should be_false
+      io.read_buffering?.should be_true
 
-      io.sync = true
-      io.sync?.should be_true
+      io.read_buffering = false
+      io.read_buffering?.should be_false
 
       byte = Bytes.new(1)
       io.read_fully(byte)
@@ -322,16 +324,16 @@ describe "IO::Buffered" do
       str.pos = 0
 
       io = BufferedWrapper.new(str)
-      io.sync?.should be_false
+      io.read_buffering?.should be_true
 
       IO::Buffered::BUFFER_SIZE.times do
         byte = Bytes.new(1)
         io.read_fully(byte)
         byte[0].should eq('a'.ord.to_u8)
       end
 
-      io.sync = true
-      io.sync?.should be_true
+      io.read_buffering = false
+      io.read_buffering?.should be_false
 
       str << "bcde"
       str.pos -= 4
@@ -347,10 +349,10 @@ describe "IO::Buffered" do
       str = IO::Memory.new "abc"
 
       io = BufferedWrapper.new(str)
-      io.sync?.should be_false
+      io.read_buffering?.should be_true
 
-      io.sync = true
-      io.sync?.should be_true
+      io.read_buffering = false
+      io.read_buffering?.should be_false
 
       io.read_byte.should eq('a'.ord.to_u8)
 
@@ -363,14 +365,14 @@ describe "IO::Buffered" do
       str.pos = 0
 
       io = BufferedWrapper.new(str)
-      io.sync?.should be_false
+      io.read_buffering?.should be_true
 
       IO::Buffered::BUFFER_SIZE.times do
         io.read_byte.should eq('a'.ord.to_u8)
       end
 
-      io.sync = true
-      io.sync?.should be_true
+      io.read_buffering = false
+      io.read_buffering?.should be_false
 
       str << "bcde"
       str.pos -= 4

diff --git a/src/crystal/system/unix/urandom.cr b/src/crystal/system/unix/urandom.cr
@@ -11,7 +11,7 @@ module Crystal::System::Random
     return unless urandom.info.type.character_device?
 
     urandom.close_on_exec = true
-    urandom.sync = true # don't buffer bytes
+    urandom.read_buffering = false
     @@urandom = urandom
   end
 

diff --git a/src/io/buffered.cr b/src/io/buffered.cr
@@ -10,6 +10,7 @@ module IO::Buffered
   @in_buffer_rem = Bytes.empty
   @out_count = 0
   @sync = false
+  @read_buffering = true
   @flush_on_newline = false
 
   # Reads at most *slice.size* bytes from the wrapped `IO` into *slice*.
@@ -33,9 +34,9 @@ module IO::Buffered
   def read_byte : UInt8?
     check_open
 
-    fill_buffer if !sync? && @in_buffer_rem.empty?
+    fill_buffer if read_buffering? && @in_buffer_rem.empty?
     if @in_buffer_rem.empty?
-      return nil unless sync?
+      return nil if read_buffering?
 
       byte = uninitialized UInt8
       if read(Slice.new(pointerof(byte), 1)) == 1
@@ -61,7 +62,7 @@ module IO::Buffered
       # If we are asked to read more than half the buffer's size,
       # read directly into the slice, as it's not worth the extra
       # memory copy.
-      if sync? || count >= BUFFER_SIZE / 2
+      if !read_buffering? || count >= BUFFER_SIZE / 2
         return unbuffered_read(slice[0, count]).to_i
       else
         fill_buffer
@@ -175,19 +176,29 @@ module IO::Buffered
     @flush_on_newline
   end
 
-  # Turns on/off `IO` buffering. When *sync* is set to `true`, no buffering
+  # Turns on/off `IO` **write** buffering. When *sync* is set to `true`, no buffering
   # will be done (that is, writing to this `IO` is immediately synced to the
   # underlying `IO`).
   def sync=(sync)
     flush if sync && !@sync
     @sync = !!sync
   end
 
-  # Determines if this `IO` does buffering. If `true`, no buffering is done.
+  # Determines if this `IO` does write buffering. If `true`, no buffering is done.
   def sync?
     @sync
   end
 
+  # Turns on/off `IO` **read** buffering.
+  def read_buffering=(read_buffering)
+    @read_buffering = !!read_buffering
+  end
+
+  # Determines whether this `IO` buffers reads.
+  def read_buffering?
+    @read_buffering
+  end
+
   # Flushes any buffered data and the underlying `IO`. Returns `self`.
   def flush
     unbuffered_write(Slice.new(out_buffer, @out_count)) if @out_count > 0