Increase test coverage

Makefile

@@ -13,26 +13,17 @@ docs:
 	python3 custom-pydoc.py > docs/pycdlib-api.html
 
 flake8:
-	-flake8-3 --ignore=E501,E266 --max-complexity 80 pycdlib tools/*
-
-# kernprof-3 comes from the "line_profiler" package.  It allows performance
-# profiling on a line-by-line basis, but needs to be told which functions to
-# profile by using an "@profile" decorator on particular functions.  The easiest
-# way to use this is to profile using the built-in cProfile module (like the
-# "profile" target), then mark the hotspots with "@profile", and then run
-# the "lineprof" target.
-lineprof:
-	kernprof-3 -v -l /usr/bin/py.test-3 --verbose tests
+	-python3 -m flake8 --ignore=E501,E266 --max-complexity 80 pycdlib tools/*
 
 mypy:
-	mypy --ignore-missing-imports -p pycdlib
+	python3 -m mypy --ignore-missing-imports -p pycdlib
 
 profile:
-	python3 -m cProfile -o profile /usr/bin/py.test-3 --verbose tests
+	python3 -m cProfile -o profile -m pytest --verbose tests
 	python3 -c "import pstats; p=pstats.Stats('profile');p.strip_dirs();p.sort_stats('time').print_stats(30)"
 
 pylint:
-	-pylint-3 --rcfile=pylint.conf pycdlib tools/*
+	-python3 -m pylint --rcfile=pylint.conf pycdlib tools/*
 
 rpm: sdist
 	rpmbuild -ba python-pycdlib.spec --define "_sourcedir `pwd`/dist"
@@ -41,17 +32,17 @@ sdist:
 	python3 setup.py sdist
 
 slowtests:
-	PYCDLIB_TRACK_WRITES=1 py.test-3 --basetemp=/var/tmp/pycdlib-tests --runslow --verbose tests
+	PYCDLIB_TRACK_WRITES=1 python3 -m pytest --basetemp=/var/tmp/pycdlib-tests --runslow --verbose tests
 
 srpm: sdist
 	rpmbuild -bs python-pycdlib.spec --define "_sourcedir `pwd`/dist"
 
 test-coverage:
-	PYCDLIB_TRACK_WRITES=1 coverage3 run --source pycdlib /usr/bin/py.test-3 --basetemp=/var/tmp/pycdlib-tests --runslow --verbose tests
-	coverage3 html
+	PYCDLIB_TRACK_WRITES=1 python3 -m coverage run --source pycdlib -m pytest --basetemp=/var/tmp/pycdlib-tests --runslow --verbose tests
+	python3 -m coverage html
 	xdg-open htmlcov/index.html
 
 tests:
-	py.test-3 --verbose tests
+	python3 -m pytest --verbose tests
 
 .PHONY: clean deb docs flake8 lineprof mypy profile pylint rpm sdist slowtests srpm test-coverage tests

pycdlib/eltorito.py

@@ -164,13 +164,11 @@ def _checksum(data):
         Returns:
          The checksum of the data.
         """
-        if isinstance(data, bytes):
-            myord = int
-        else:
-            myord = ord
         csum = 0
         for i, val in enumerate(data):
-            short = (myord(val) << (8 * (i % 2))) & 0xffff
+            # Iterating over a bytes object yields integers, so no conversion
+            # is needed here.
+            short = (val << (8 * (i % 2))) & 0xffff
             # Because we are looping through bytes, and because we know the
             # above conversion can shift us up by a maximum of 8 bits, we know
             # that the sign bit is at offset 15.  If it is signed, OR with

pycdlib/inode.py

@@ -49,6 +49,11 @@ def __init__(self):
         self.num_udf = 0
         self.boot_info_table = None  # type: Optional[eltorito.EltoritoBootInfoTable]
         self.new_extent_loc = -1
+        # For an Inode created with new(), there is no original extent
+        # location, so we leave it at None (the same convention that
+        # dr.DirectoryRecord uses).  It stays None until a reshuffle assigns
+        # new_extent_loc.
+        self.orig_extent_loc = None  # type: Optional[int]
 
     def new(self, length, fp, manage_fp, offset):
         # type: (int, Union[IO[Any], str], bool, int) -> None
@@ -109,9 +114,14 @@ def extent_location(self):
         if not self._initialized:
             raise pycdlibexception.PyCdlibInternalError('Inode is not initialized')
 
-        if self.new_extent_loc < 0:
-            return self.orig_extent_loc
-        return self.new_extent_loc
+        # If a new extent location has been assigned, always prefer that one.
+        if self.new_extent_loc >= 0:
+            return self.new_extent_loc
+
+        if self.orig_extent_loc is None:
+            raise pycdlibexception.PyCdlibInternalError('Inode does not yet have an extent location assigned')
+
+        return self.orig_extent_loc
 
     def set_extent_location(self, extent):
         # type: (int) -> None

pycdlib/pycdlib.py

@@ -4516,6 +4516,14 @@ def get_file_byte_extents(self, **kwargs):
         if num_paths != 1:
             raise pycdlibexception.PyCdlibInvalidInput("Exactly one of 'iso_path', 'rr_path', 'joliet_path', or 'udf_path' must be passed")
 
+        # The byte offsets we are about to compute come from the extent
+        # locations, which are only assigned during a reshuffle.  On an ISO
+        # built with new() that has not been written out yet, no reshuffle has
+        # happened, so force one here rather than reporting offsets from
+        # unassigned extents.
+        if self._needs_reshuffle:
+            self._reshuffle_extents()
+
         extents = []  # type: List[Tuple[int, int]]
 
         if udf_path is not None:

tests/integration/test_hybrid.py

@@ -3040,3 +3040,59 @@ def test_hybrid_boot_record_retain_system_use(tmpdir):
     iso.close()
 
 # FIXME: write tests for 'empty' UDF File Entries (like on the Win2k8 ISO).
+
+def _make_zeroed_udf_file_entry_iso(tmpdir, name):
+    # Build a UDF ISO with a single file and then zero out that file's UDF
+    # File Entry, which is how an 'empty' UDF File Entry is reached.
+    indir = tmpdir.mkdir(name)
+    outfile = str(indir)+'.iso'
+
+    with open(os.path.join(str(indir), 'foo'), 'wb') as outfp:
+        outfp.write(b'foo\n')
+
+    subprocess.call(['genisoimage', '-v', '-v', '-no-pad', '-iso-level', '3',
+                     '-udf', '-o', str(outfile), str(indir)])
+
+    with open(str(outfile), 'r+b') as fp:
+        fp.seek(261*2048)
+        fp.write(b'\x00'*2048)
+
+    iso = pycdlib.PyCdlib()
+    iso.open(str(outfile))
+    return iso
+
+def test_hybrid_udf_get_file_byte_extents_zero_udf_file_entry(tmpdir):
+    iso = _make_zeroed_udf_file_entry_iso(tmpdir, 'udfextentszero')
+
+    with pytest.raises(pycdlib.pycdlibexception.PyCdlibInvalidInput) as excinfo:
+        iso.get_file_byte_extents(udf_path='/foo')
+    assert(str(excinfo.value) == 'Cannot get extents for an empty UDF File Entry')
+
+    iso.close()
+
+def test_hybrid_udf_walk_zero_udf_file_entry(tmpdir):
+    iso = _make_zeroed_udf_file_entry_iso(tmpdir, 'udfwalkzero')
+
+    with pytest.raises(pycdlib.pycdlibexception.PyCdlibInvalidInput) as excinfo:
+        list(iso.walk(udf_path='/foo'))
+    assert(str(excinfo.value) == 'Cannot get entry for empty UDF File Entry')
+
+    iso.close()
+
+def test_hybrid_udf_open_file_from_iso_zero_udf_file_entry(tmpdir):
+    iso = _make_zeroed_udf_file_entry_iso(tmpdir, 'udfopenzero')
+
+    with pytest.raises(pycdlib.pycdlibexception.PyCdlibInvalidInput) as excinfo:
+        iso.open_file_from_iso(udf_path='/foo')
+    assert(str(excinfo.value) == 'Cannot get entry for empty UDF File Entry')
+
+    iso.close()
+
+def test_hybrid_udf_add_hard_link_zero_udf_file_entry(tmpdir):
+    iso = _make_zeroed_udf_file_entry_iso(tmpdir, 'udfhardlinkzero')
+
+    with pytest.raises(pycdlib.pycdlibexception.PyCdlibInvalidInput) as excinfo:
+        iso.add_hard_link(udf_old_path='/foo', iso_new_path='/BAR.;1')
+    assert(str(excinfo.value) == 'Cannot make hard link to a UDF file with an empty UDF File Entry')
+
+    iso.close()