No unnecessary strings on POSIX header creation

The POSIX header creation methods created a new 512-byte string of NULL
bytes just to see if the header was an empty header. This could be simplified
with a single 512-byte constant string for comparison, but a short-circuiting
comparison will be more efficient over-all. Use the #each_byte enumerator with
the #any? test method on #nonzero? (data.each_byte.any?(&:nonzero?)).

Author: halostatue

.travis.yml

@@ -12,10 +12,8 @@ rvm:
   - ree
   - ruby-head
   - jruby-19mode
-  - jruby-head
 matrix:
   allow_failures:
-    - rvm: jruby-head
     - rvm: ruby-head
     - rvm: 1.8.7
     - rvm: ree

Gemfile

@@ -1,27 +1,29 @@
 # -*- ruby -*-
 
 # NOTE: This file is present to keep Travis CI happy. Edits to it will not
-# be accepted.
+# be accepted, except to remove all of this crap later.
 
 source 'https://rubygems.org/'
 
 mime_version =
   if RUBY_VERSION < '1.9'
+    gem 'rdoc', '< 4.0'
+    # gem 'ruby-debug'
     '1.25'
   elsif RUBY_VERSION < '2.0'
+    # gem 'debugger' if RUBY_ENGINE == 'ruby'
     '2.0'
-  else
+  elsif RUBY_VERSION >= '2.0'
+    if RUBY_ENGINE == 'ruby'
+      # gem 'byebug'
+      gem 'simplecov', '~> 0.7'
+      gem 'coveralls', '~> 0.7'
+    end
     '3.0'
   end
 
 gem 'mime-types', "~> #{mime_version}"
 
-if RUBY_VERSION >= '2.0' && RUBY_ENGINE == 'ruby'
-  gem 'byebug'
-  gem 'simplecov', '~> 0.7'
-  gem 'coveralls', '~> 0.7'
-end
-
 gemspec :name => 'minitar'
 
 # vim: syntax=ruby

Manifest.txt

@@ -16,6 +16,7 @@ lib/archive/tar/minitar/reader.rb
 lib/archive/tar/minitar/writer.rb
 lib/minitar.rb
 test/minitest_helper.rb
+test/support/tar_test_helpers.rb
 test/test_tar_header.rb
 test/test_tar_input.rb
 test/test_tar_output.rb

Rakefile

@@ -28,6 +28,7 @@ spec = Hoe.spec 'minitar' do
   extra_dev_deps << ['minitest', '~> 5.3']
   extra_dev_deps << ['minitest-autotest', ['>= 1.0.b', '<2']]
   extra_dev_deps << ['rake', '~> 10.0']
+  extra_dev_deps << ['rdoc', '>= 0.0']
 end
 
 if RUBY_VERSION >= '2.0' && RUBY_ENGINE == 'ruby'

lib/archive/tar/minitar/posix_header.rb

@@ -32,6 +32,8 @@ module Archive::Tar::Minitar; end
 # POSIX indicates that "A POSIX-compliant implementation must treat any
 # unrecognized typeflag value as a regular file."
 class Archive::Tar::Minitar::PosixHeader
+  BLOCK_SIZE = 512
+
   # Fields that must be set in a POSIX tar(1) header.
   REQUIRED_FIELDS = [ :name, :size, :prefix, :mode ].freeze
   # Fields that may be set in a POSIX tar(1) header.
@@ -46,7 +48,7 @@ class Archive::Tar::Minitar::PosixHeader
   FIELDS.each { |f| attr_reader f.to_sym unless f.to_sym == :name }
 
   # The name of the file. By default, limited to 100 bytes. Required. May be
-  # longer (up to 512 bytes) if using the GNU long name tar extension.
+  # longer (up to BLOCK_SIZE bytes) if using the GNU long name tar extension.
   attr_accessor :name
 
   # The pack format passed to Array#pack for encoding a header.
@@ -57,7 +59,7 @@ class Archive::Tar::Minitar::PosixHeader
   class << self
     # Creates a new PosixHeader from a data stream.
     def from_stream(stream)
-      from_data(stream.read(512))
+      from_data(stream.read(BLOCK_SIZE))
     end
 
     # Creates a new PosixHeader from a data stream. Deprecated; use
@@ -67,7 +69,7 @@ def new_from_stream(stream)
       from_stream(stream)
     end
 
-    # Creates a new PosixHeader from a 512-byte data buffer.
+    # Creates a new PosixHeader from a BLOCK_SIZE-byte data buffer.
     def from_data(data)
       fields    = data.unpack(HEADER_UNPACK_FORMAT)
       name      = fields.shift
@@ -87,7 +89,7 @@ def from_data(data)
       devminor  = fields.shift.oct
       prefix    = fields.shift
 
-      empty = (data == "\0" * 512)
+      empty = !data.each_byte.any?(&:nonzero?)
 
       new(
         :name => name,
@@ -171,7 +173,7 @@ def header(chksum)
            oct(mtime, 11), chksum, " ", typeflag, linkname, magic, version,
            uname, gname, oct(devmajor, 7), oct(devminor, 7), prefix]
     str = arr.pack(HEADER_PACK_FORMAT)
-    str + "\0" * ((512 - str.size) % 512)
+    str + "\0" * ((BLOCK_SIZE - str.size) % BLOCK_SIZE)
   end
 
   ##

minitar.gemspec

@@ -2,60 +2,60 @@
 # stub: minitar 0.6 ruby lib
 
 Gem::Specification.new do |s|
-  s.name = "minitar".freeze
+  s.name = "minitar"
   s.version = "0.6"
 
-  s.required_rubygems_version = Gem::Requirement.new(">= 0".freeze) if s.respond_to? :required_rubygems_version=
-  s.require_paths = ["lib".freeze]
-  s.authors = ["Austin Ziegler".freeze]
-  s.date = "2016-11-04"
-  s.description = "The minitar library is a pure-Ruby library that provides the ability to deal\nwith POSIX tar(1) archive files.\n\nThis is release 0.6, \u{2026}\n\nminitar (previously called Archive::Tar::Minitar) is based heavily on code\noriginally written by Mauricio Julio Fern\u{e1}ndez Pradier for the rpa-base\nproject.".freeze
-  s.email = ["[email protected]".freeze]
-  s.extra_rdoc_files = ["Contributing.md".freeze, "History.md".freeze, "Licence.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "docs/bsdl.txt".freeze, "docs/ruby.txt".freeze]
-  s.files = ["Contributing.md".freeze, "History.md".freeze, "Licence.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "Rakefile".freeze, "docs/bsdl.txt".freeze, "docs/ruby.txt".freeze, "lib/archive-tar-minitar.rb".freeze, "lib/archive/tar/minitar.rb".freeze, "lib/archive/tar/minitar/input.rb".freeze, "lib/archive/tar/minitar/output.rb".freeze, "lib/archive/tar/minitar/posix_header.rb".freeze, "lib/archive/tar/minitar/reader.rb".freeze, "lib/archive/tar/minitar/writer.rb".freeze, "lib/minitar.rb".freeze, "test/minitest_helper.rb".freeze, "test/test_tar_header.rb".freeze, "test/test_tar_input.rb".freeze, "test/test_tar_output.rb".freeze, "test/test_tar_reader.rb".freeze, "test/test_tar_writer.rb".freeze]
-  s.homepage = "https://github.com/halostatue/minitar/".freeze
-  s.licenses = ["Ruby".freeze, "BSD-2-Clause".freeze]
-  s.rdoc_options = ["--main".freeze, "README.rdoc".freeze]
-  s.required_ruby_version = Gem::Requirement.new(">= 1.8".freeze)
-  s.rubygems_version = "2.6.6".freeze
-  s.summary = "The minitar library is a pure-Ruby library that provides the ability to deal with POSIX tar(1) archive files".freeze
+  s.required_rubygems_version = Gem::Requirement.new(">= 0") if s.respond_to? :required_rubygems_version=
+  s.require_paths = ["lib"]
+  s.authors = ["Austin Ziegler"]
+  s.date = "2016-11-08"
+  s.description = "The minitar library is a pure-Ruby library that provides the ability to deal\nwith POSIX tar(1) archive files.\n\nThis is release 0.6, \u{2026}\n\nminitar (previously called Archive::Tar::Minitar) is based heavily on code\noriginally written by Mauricio Julio Fern\u{e1}ndez Pradier for the rpa-base\nproject."
+  s.email = ["[email protected]"]
+  s.extra_rdoc_files = ["Code-of-Conduct.md", "Contributing.md", "History.md", "Licence.md", "Manifest.txt", "README.rdoc", "docs/bsdl.txt", "docs/ruby.txt"]
+  s.files = ["Code-of-Conduct.md", "Contributing.md", "History.md", "Licence.md", "Manifest.txt", "README.rdoc", "Rakefile", "docs/bsdl.txt", "docs/ruby.txt", "lib/archive-tar-minitar.rb", "lib/archive/tar/minitar.rb", "lib/archive/tar/minitar/input.rb", "lib/archive/tar/minitar/output.rb", "lib/archive/tar/minitar/posix_header.rb", "lib/archive/tar/minitar/reader.rb", "lib/archive/tar/minitar/writer.rb", "lib/minitar.rb", "test/minitest_helper.rb", "test/support/tar_test_helpers.rb", "test/test_tar_header.rb", "test/test_tar_input.rb", "test/test_tar_output.rb", "test/test_tar_reader.rb", "test/test_tar_writer.rb"]
+  s.homepage = "https://github.com/halostatue/minitar/"
+  s.licenses = ["Ruby", "BSD-2-Clause"]
+  s.rdoc_options = ["--main", "README.rdoc"]
+  s.required_ruby_version = Gem::Requirement.new(">= 1.8")
+  s.rubygems_version = "2.5.1"
+  s.summary = "The minitar library is a pure-Ruby library that provides the ability to deal with POSIX tar(1) archive files"
 
   if s.respond_to? :specification_version then
     s.specification_version = 4
 
     if Gem::Version.new(Gem::VERSION) >= Gem::Version.new('1.2.0') then
-      s.add_development_dependency(%q<minitest>.freeze, ["~> 5.9"])
-      s.add_development_dependency(%q<hoe-doofus>.freeze, ["~> 1.0"])
-      s.add_development_dependency(%q<hoe-gemspec2>.freeze, ["~> 1.1"])
-      s.add_development_dependency(%q<hoe-git>.freeze, ["~> 1.6"])
-      s.add_development_dependency(%q<hoe-rubygems>.freeze, ["~> 1.0"])
-      s.add_development_dependency(%q<hoe-travis>.freeze, ["~> 1.2"])
-      s.add_development_dependency(%q<minitest-autotest>.freeze, ["< 2", ">= 1.0.b"])
-      s.add_development_dependency(%q<rake>.freeze, ["~> 10.0"])
-      s.add_development_dependency(%q<rdoc>.freeze, ["~> 4.0"])
-      s.add_development_dependency(%q<hoe>.freeze, ["~> 3.15"])
+      s.add_development_dependency(%q<minitest>, ["~> 5.9"])
+      s.add_development_dependency(%q<hoe-doofus>, ["~> 1.0"])
+      s.add_development_dependency(%q<hoe-gemspec2>, ["~> 1.1"])
+      s.add_development_dependency(%q<hoe-git>, ["~> 1.6"])
+      s.add_development_dependency(%q<hoe-rubygems>, ["~> 1.0"])
+      s.add_development_dependency(%q<hoe-travis>, ["~> 1.2"])
+      s.add_development_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
+      s.add_development_dependency(%q<rake>, ["~> 10.0"])
+      s.add_development_dependency(%q<rdoc>, [">= 0.0"])
+      s.add_development_dependency(%q<hoe>, ["~> 3.15"])
     else
-      s.add_dependency(%q<minitest>.freeze, ["~> 5.9"])
-      s.add_dependency(%q<hoe-doofus>.freeze, ["~> 1.0"])
-      s.add_dependency(%q<hoe-gemspec2>.freeze, ["~> 1.1"])
-      s.add_dependency(%q<hoe-git>.freeze, ["~> 1.6"])
-      s.add_dependency(%q<hoe-rubygems>.freeze, ["~> 1.0"])
-      s.add_dependency(%q<hoe-travis>.freeze, ["~> 1.2"])
-      s.add_dependency(%q<minitest-autotest>.freeze, ["< 2", ">= 1.0.b"])
-      s.add_dependency(%q<rake>.freeze, ["~> 10.0"])
-      s.add_dependency(%q<rdoc>.freeze, ["~> 4.0"])
-      s.add_dependency(%q<hoe>.freeze, ["~> 3.15"])
+      s.add_dependency(%q<minitest>, ["~> 5.9"])
+      s.add_dependency(%q<hoe-doofus>, ["~> 1.0"])
+      s.add_dependency(%q<hoe-gemspec2>, ["~> 1.1"])
+      s.add_dependency(%q<hoe-git>, ["~> 1.6"])
+      s.add_dependency(%q<hoe-rubygems>, ["~> 1.0"])
+      s.add_dependency(%q<hoe-travis>, ["~> 1.2"])
+      s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
+      s.add_dependency(%q<rake>, ["~> 10.0"])
+      s.add_dependency(%q<rdoc>, [">= 0.0"])
+      s.add_dependency(%q<hoe>, ["~> 3.15"])
     end
   else
-    s.add_dependency(%q<minitest>.freeze, ["~> 5.9"])
-    s.add_dependency(%q<hoe-doofus>.freeze, ["~> 1.0"])
-    s.add_dependency(%q<hoe-gemspec2>.freeze, ["~> 1.1"])
-    s.add_dependency(%q<hoe-git>.freeze, ["~> 1.6"])
-    s.add_dependency(%q<hoe-rubygems>.freeze, ["~> 1.0"])
-    s.add_dependency(%q<hoe-travis>.freeze, ["~> 1.2"])
-    s.add_dependency(%q<minitest-autotest>.freeze, ["< 2", ">= 1.0.b"])
-    s.add_dependency(%q<rake>.freeze, ["~> 10.0"])
-    s.add_dependency(%q<rdoc>.freeze, ["~> 4.0"])
-    s.add_dependency(%q<hoe>.freeze, ["~> 3.15"])
+    s.add_dependency(%q<minitest>, ["~> 5.9"])
+    s.add_dependency(%q<hoe-doofus>, ["~> 1.0"])
+    s.add_dependency(%q<hoe-gemspec2>, ["~> 1.1"])
+    s.add_dependency(%q<hoe-git>, ["~> 1.6"])
+    s.add_dependency(%q<hoe-rubygems>, ["~> 1.0"])
+    s.add_dependency(%q<hoe-travis>, ["~> 1.2"])
+    s.add_dependency(%q<minitest-autotest>, ["< 2", ">= 1.0.b"])
+    s.add_dependency(%q<rake>, ["~> 10.0"])
+    s.add_dependency(%q<rdoc>, [">= 0.0"])
+    s.add_dependency(%q<hoe>, ["~> 3.15"])
   end
 end

test/minitest_helper.rb

@@ -6,89 +6,9 @@
 gem 'minitest'
 require 'minitest/autorun'
 
-module TarTester
-  private
-  def assert_headers_equal(h1, h2)
-    fields = %w(name 100 mode 8 uid 8 gid 8 size 12 mtime 12 checksum 8
-                typeflag 1 linkname 100 magic 6 version 2 uname 32 gname 32
-                devmajor 8 devminor 8 prefix 155)
-    offset = 0
-    until fields.empty?
-      name = fields.shift
-      length = fields.shift.to_i
-      if name == "checksum"
-        chksum_off = offset
-        offset += length
-        next
-      end
-      assert_equal(h1[offset, length], h2[offset, length],
-                   "Field #{name} of the tar header differs.")
-      offset += length
-    end
-    assert_equal(h1[chksum_off, 8], h2[chksum_off, 8], "Checksumes differ.")
-  end
-
-  def assert_modes_equal(expected, actual, name)
-    unless Minitar.windows?
-      expected = "%04o" % (expected & 0777)
-      actual = "%04o" % (actual & 0777)
-
-      assert_equal(expected, actual, "Mode for #{name} does not match")
-    end
-  end
-
-  def tar_file_header(fname, dname, mode, length)
-    h = header("0", fname, dname, length, mode)
-    checksum = calc_checksum(h)
-    header("0", fname, dname, length, mode, checksum)
-  end
-
-  def tar_dir_header(name, prefix, mode)
-    h = header("5", name, prefix, 0, mode)
-    checksum = calc_checksum(h)
-    header("5", name, prefix, 0, mode, checksum)
-  end
-
-  def header(type, fname, dname, length, mode, checksum = nil)
-    checksum ||= " " * 8
-    arr = [ASCIIZ(fname, 100), Z(to_oct(mode, 7)), Z(to_oct(nil, 7)),
-           Z(to_oct(nil, 7)), Z(to_oct(length, 11)), Z(to_oct(0, 11)),
-           checksum, type, "\0" * 100, "ustar\0", "00", ASCIIZ("", 32),
-           ASCIIZ("", 32), Z(to_oct(nil, 7)), Z(to_oct(nil, 7)),
-           ASCIIZ(dname, 155) ]
-    arr = arr.join.bytes.to_a
-    h = arr.pack("C100C8C8C8C12C12C8CC100C6C2C32C32C8C8C155")
-    ret = h + "\0" * (512 - h.size)
-    assert_equal(512, ret.size)
-    ret
-  end
-
-  def calc_checksum(header)
-    sum = header.unpack("C*").inject { |s, a| s + a }
-    SP(Z(to_oct(sum, 6)))
-  end
-
-  def to_oct(n, pad_size)
-    if n.nil?
-      "\0" * pad_size
-    else
-      "%0#{pad_size}o" % n
-    end
-  end
-
-  def ASCIIZ(str, length)
-    str + "\0" * (length - str.length)
-  end
-
-  def SP(s)
-    s + " "
-  end
-
-  def Z(s)
-    s + "\0"
-  end
+Dir.glob(File.join(File.dirname(__FILE__), 'support/*.rb')).each do |support|
+  require support
+end
 
-  def SP_Z(s)
-    s + " \0"
-  end
+module TarTester
 end