add io.readChars overload (simpler, less error prone) (#16044)

* add simpler to use readChars overload

* use new readChars overload

* Update lib/wrappers/openssl.nim

Co-authored-by: Andreas Rumpf <[email protected]>
Co-authored-by: flywind <[email protected]>

changelog.md

@@ -66,6 +66,8 @@
 - `os.FileInfo` (returned by `getFileInfo`) now contains `blockSize`,
   determining preferred I/O block size for this file object.
 
+- Added a simpler to use `io.readChars` overload.
+
 - `repr` now doesn't insert trailing newline; previous behavior was very inconsistent,
   see #16034. Use `-d:nimLegacyReprWithNewline` for previous behavior.
 
@@ -167,6 +169,7 @@ provided by the operating system.
   dumping (on select signals) and notifying the parent process about the cause
   of termination.
 
+
 ## Language changes
 
 - `nimscript` now handles `except Exception as e`.

lib/std/sha1.nim

@@ -231,7 +231,7 @@ proc secureHashFile*(filename: string): SecureHash =
   var state = newSha1State()
   var buffer = newString(BufferLength)
   while true:
-    let length = readChars(f, buffer, 0, BufferLength)
+    let length = readChars(f, buffer)
     if length == 0:
       break
     buffer.setLen(length)

lib/system/io.nim

@@ -174,14 +174,18 @@ proc readBytes*(f: File, a: var openArray[int8|uint8], start, len: Natural): int
   ## `len` (if not as many bytes are remaining), but not greater.
   result = readBuffer(f, addr(a[start]), len)
 
+proc readChars*(f: File, a: var openArray[char]): int {.tags: [ReadIOEffect], benign.} =
+  ## reads up to `a.len` bytes into the buffer `a`. Returns
+  ## the actual number of bytes that have been read which may be less than
+  ## `a.len` (if not as many bytes are remaining), but not greater.
+  result = readBuffer(f, addr(a[0]), a.len)
+
 proc readChars*(f: File, a: var openArray[char], start, len: Natural): int {.
-  tags: [ReadIOEffect], benign.} =
+  tags: [ReadIOEffect], benign, deprecated:
+    "use other `readChars` overload, possibly via: readChars(toOpenArray(buf, start, len-1))".} =
   ## reads `len` bytes into the buffer `a` starting at ``a[start]``. Returns
   ## the actual number of bytes that have been read which may be less than
   ## `len` (if not as many bytes are remaining), but not greater.
-  ##
-  ## **Warning:** The buffer `a` must be pre-allocated. This can be done
-  ## using, for example, ``newString``.
   if (start + len) > len(a):
     raiseEIO("buffer overflow: (start+len) > length of openarray buffer")
   result = readBuffer(f, addr(a[start]), len)

lib/wrappers/openssl.nim

@@ -760,7 +760,7 @@ proc md5_File*(file: string): string {.raises: [IOError,Exception].} =
     ctx: MD5_CTX
 
   discard md5_Init(ctx)
-  while(let bytes = f.readChars(buf, 0, sz); bytes > 0):
+  while (let bytes = f.readChars(buf); bytes > 0):
     discard md5_Update(ctx, buf[0].addr, cast[csize_t](bytes))
 
   discard md5_Final(buf[0].addr, ctx)

tests/stdlib/tio.nim

@@ -0,0 +1,37 @@
+# xxx move to here other tests that belong here; io is a proper module
+
+import std/os
+from stdtest/specialpaths import buildDir
+
+block: # readChars
+  let file = buildDir / "D20201118T205105.txt"
+  let s = "he\0l\0lo"
+  writeFile(file, s)
+  defer: removeFile(file)
+  let f = open(file)
+  defer: close(f)
+  let n = f.getFileInfo.blockSize
+  var buf = newString(n)
+  template fn =
+    let n2 = f.readChars(buf)
+    doAssert n2 == s.len
+    doAssert buf[0..<n2] == s
+  fn()
+  setFilePos(f, 0)
+  fn()
+
+  block:
+    setFilePos(f, 0)
+    var s2: string
+    let nSmall = 2
+    for ai in buf.mitems: ai = '\0'
+    var n2s: seq[int]
+    while true:
+      let n2 = f.readChars(toOpenArray(buf, 0, nSmall-1))
+      # xxx: maybe we could support: toOpenArray(buf, 0..nSmall)
+      n2s.add n2
+      s2.add buf[0..<n2]
+      if n2 == 0:
+        break
+    doAssert n2s == @[2,2,2,1,0]
+    doAssert s2 == s