chore: track fs state on WriteEntry class, not in arguments

lib/write-entry.js

@@ -52,6 +52,15 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
     this.noMtime = !!opt.noMtime
     this.mtime = opt.mtime || null
 
+    this.fd = null
+    this.blockLen = null
+    this.blockRemain = null
+    this.buf = null
+    this.offset = null
+    this.length = null
+    this.pos = null
+    this.remain = null
+
     if (typeof opt.onwarn === 'function')
       this.on('warn', opt.onwarn)
 
@@ -208,75 +217,89 @@ const WriteEntry = warner(class WriteEntry extends MiniPass {
   }
 
   [ONOPENFILE] (fd) {
-    const blockLen = 512 * Math.ceil(this.stat.size / 512)
-    const bufLen = Math.min(blockLen, this.maxReadSize)
-    const buf = Buffer.allocUnsafe(bufLen)
-    this[READ](fd, buf, 0, buf.length, 0, this.stat.size, blockLen)
+    this.fd = fd
+    this.blockLen = 512 * Math.ceil(this.stat.size / 512)
+    this.blockRemain = this.blockLen
+    const bufLen = Math.min(this.blockLen, this.maxReadSize)
+    this.buf = Buffer.allocUnsafe(bufLen)
+    this.offset = 0
+    this.pos = 0
+    this.remain = this.stat.size
+    this.length = this.buf.length
+    this[READ](this.stat.size)
   }
 
-  [READ] (fd, buf, offset, length, pos, remain, blockRemain) {
+  [READ] () {
+    const { fd, buf, offset, length, pos } = this
     fs.read(fd, buf, offset, length, pos, (er, bytesRead) => {
-      if (er)
-        return this[CLOSE](fd, _ => this.emit('error', er))
-      this[ONREAD](fd, buf, offset, length, pos, remain, blockRemain, bytesRead)
+      if (er) {
+        // ignoring the error from close(2) is a bad practice, but at
+        // this point we already have an error, don't need another one
+        return this[CLOSE](() => this.emit('error', er))
+      }
+      this[ONREAD](bytesRead)
     })
   }
 
-  [CLOSE] (fd, cb) {
-    fs.close(fd, cb)
+  [CLOSE] (cb) {
+    fs.close(this.fd, cb)
   }
 
-  [ONREAD] (fd, buf, offset, length, pos, remain, blockRemain, bytesRead) {
-    if (bytesRead <= 0 && remain > 0) {
+  [ONREAD] (bytesRead) {
+    if (bytesRead <= 0 && this.remain > 0) {
       const er = new Error('encountered unexpected EOF')
       er.path = this.absolute
       er.syscall = 'read'
       er.code = 'EOF'
-      this[CLOSE](fd, _ => _)
-      return this.emit('error', er)
+      return this[CLOSE](() => this.emit('error', er))
     }
 
-    if (bytesRead > remain) {
+    if (bytesRead > this.remain) {
       const er = new Error('did not encounter expected EOF')
       er.path = this.absolute
       er.syscall = 'read'
       er.code = 'EOF'
-      this[CLOSE](fd, _ => _)
-      return this.emit('error', er)
+      return this[CLOSE](() => this.emit('error', er))
     }
 
     // null out the rest of the buffer, if we could fit the block padding
-    if (bytesRead === remain) {
-      for (let i = bytesRead; i < length && bytesRead < blockRemain; i++) {
-        buf[i + offset] = 0
-        bytesRead ++
-        remain ++
+    // at the end of this loop, we've incremented bytesRead and this.remain
+    // to be incremented up to the blockRemain level, as if we had expected
+    // to get a null-padded file, and read it until the end.  then we will
+    // decrement both remain and blockRemain by bytesRead, and know that we
+    // reached the expected EOF, without any null buffer to append.
+    if (bytesRead === this.remain) {
+      for (let i = bytesRead; i < this.length && bytesRead < this.blockRemain; i++) {
+        this.buf[i + this.offset] = 0
+        bytesRead++
+        this.remain++
       }
     }
 
-    const writeBuf = offset === 0 && bytesRead === buf.length ?
-      buf : buf.slice(offset, offset + bytesRead)
-    remain -= bytesRead
-    blockRemain -= bytesRead
-    pos += bytesRead
-    offset += bytesRead
+    const writeBuf = this.offset === 0 && bytesRead === this.buf.length ?
+      this.buf : this.buf.slice(this.offset, this.offset + bytesRead)
+    this.remain -= bytesRead
+    this.blockRemain -= bytesRead
+    this.pos += bytesRead
+    this.offset += bytesRead
 
     this.write(writeBuf)
 
-    if (!remain) {
-      if (blockRemain)
-        this.write(Buffer.alloc(blockRemain))
-      this.end()
-      this[CLOSE](fd, _ => _)
-      return
+    if (!this.remain) {
+      if (this.blockRemain)
+        this.write(Buffer.alloc(this.blockRemain))
+      return this[CLOSE](/* istanbul ignore next - legacy */
+        er => er ? this.emit('error', er) : this.end())
     }
 
-    if (offset >= length) {
-      buf = Buffer.allocUnsafe(length)
-      offset = 0
+    if (this.offset >= this.length) {
+      // if we only have a smaller bit left to read, alloc a smaller buffer
+      // otherwise, keep it the same length it was before.
+      this.buf = Buffer.allocUnsafe(Math.min(this.blockRemain, this.buf.length))
+      this.offset = 0
     }
-    length = buf.length - offset
-    this[READ](fd, buf, offset, length, pos, remain, blockRemain)
+    this.length = this.buf.length - this.offset
+    this[READ]()
   }
 })
 
@@ -297,20 +320,27 @@ class WriteEntrySync extends WriteEntry {
     this[ONOPENFILE](fs.openSync(this.absolute, 'r'))
   }
 
-  [READ] (fd, buf, offset, length, pos, remain, blockRemain) {
+  [READ] () {
     let threw = true
     try {
+      const { fd, buf, offset, length, pos } = this
       const bytesRead = fs.readSync(fd, buf, offset, length, pos)
-      this[ONREAD](fd, buf, offset, length, pos, remain, blockRemain, bytesRead)
+      this[ONREAD](bytesRead)
       threw = false
     } finally {
-      if (threw)
-        try { this[CLOSE](fd) } catch (er) {}
+      // ignoring the error from close(2) is a bad practice, but at
+      // this point we already have an error, don't need another one
+      if (threw) {
+        try {
+          this[CLOSE](() => {})
+        } catch (er) {}
+      }
     }
   }
 
-  [CLOSE] (fd) {
-    fs.closeSync(fd)
+  [CLOSE] (cb) {
+    fs.closeSync(this.fd)
+    cb()
   }
 }