`Int32x32To64` macro internally truncates the arguments to int32, while
`time_t` is 64-bit on most/all modern platforms. Therefore, usage of
this macro creates a Year 2038 bug.
I detailed this issue a while ago in a writeup, and spotted the same
issue in this repository when updating the list of affected
repositories:
<https://cookieplmonster.github.io/2022/02/17/year-2038-problem/>
A few more notes:
1. I changed all uses of `Int32x32To64` en masse, even though at least
one of them was technically OK and used with int32 parameters only. IMO
better safe than sorry.
2. This is untested, but it's a small enough change that I hope the CI
success is a good enough indicator.
Previously skipped tests were reported like this when running the *_test
binaries:
```
4: test_acl_platform_nfs4 ok (S)
```
Let's make this more obvious:
```
4: test_acl_platform_nfs4 skipped
```
This plumbing is required for cmake/ctest to recognise and report
skipped tests.
Now skipped tests in cmake ci jobs are reported like so:
```
Start 7: libarchive_test_acl_platform_posix1e_read
7/785 Test #7: libarchive_test_acl_platform_posix1e_read ................................***Skipped 0.02 sec
```
And there is a list of skipped tests shown at the end of the test run.
Moving the tests' integer reading functions to test_utils so that they
all use the same as well as moving the few using the archive_endian
functions over to the test_utils helper.
Follow-up from libarchive#2390.
This fixes various code quality issues I encountered while chasing a
memory leak reported by test automation. I failed to reproduce the
memory leak, but I hope you find this useful nonetheless.
Fixes all test-related compiler warnings with Visual Studio 2022 on
Windows 11.
Contains some changes from
https://github.com/libarchive/libarchive/pull/2095.
CC: @dunhor
---------
Co-authored-by: Duncan Horn <dunhor@microsoft.com>
This significantly changes how end-of-data markers are parsed.
In particular, the spec allows the end-of-data marker to have either
32-bit or 64-bit size values, and there is basically no indication which
is being used. (The spec mentions "Zip64 mode" in many places, but there
is no definitive way for a Zip reader to know whether the writer is
using this mode or not. My mis-reading of another part of the spec
caused me to believe that the Zip64 Extra Data field was such a marker,
but I've been patiently corrected. ;-)
So a Zip reader just has to guess: Try every possible end-of-data marker
format and accept it if any of the four possible forms is correct. In
libarchive's case, this required some non-trivial additional refactoring
to ensure that the CRC32, compressed size, and uncompressed size
statistics are always updated _before_ we need to look for an
end-of-data marker.
This generally follows the strategy outlined by Mark Adler for his
`sunzip` streaming unzip implementation.
While testing this, I played with pmqs/zipdetails which pointed out a
discrepancy in how libarchive writes the `UT` extra field. I folded a
fix for that in here as well.
Resolves#1834
TODO: It would be nice to augment the test suite with some static files
created by Java's implementation to verify that we can read those when
they hold entries of +/- 4GiB. The existing
`test_write_format_zip_large` uses an ad hoc RLE encoding trick to
exercise writing and reading back multi-gigabyte entries. I wonder if
that could be generalized to support deflate-compressed Zip data stored
in test files?
The libarchive source tree is littered with `__FBSDID("$FreeBSD.*")` and
'$FreeBSD$' tags left over from extracting it from FreeBSD's Subversion
repo. They never made sense for a git repo as git doesn't expand them
and FreeBSD has now removed `$FreeBSD$` from most local source files so
these stand out.
In addition to `__FBSDID` I've removed `__RCSID` which was used once for
a `$NetBSD$` expansion. There might be more of a case to be made for
preserving this one as a diff-reduction measure, but it seems mostly
pointless.
This builds and tests pass except for
libarchive_test_read_disk_directory_traversals which failed on master as
well.
Otherwise there are 32/64-bit pointer conversions going on. In Windows
since MSVC2005, time_t has been 64-bit. MinGW needs a hack to get 64-bit
time_t.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
These were missing from 2d329073435d36065ce30bfd29428f6a45e41016
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Also changed a few vsprintf to vsnprintf.
Most cases were trivial, one private function was changed to take the buffer length, one case required some fancy arithmetic.
A few guiding principles:
* Each test source file includes ONLY "test.h" to make it easy
to create new tests.
* Each test suite has a "test.h" that includes "test_util/test_common.h"
to get access to all the common testing utility functions.
So "test_common.h" is then responsible for including
any smaller headers that declare specific pieces of
shared test functionality.
I've also pulled some test filtering logic that was _only_ used
in test_main.c into that file, and repurposed "test_utils.[ch]"
for common utility code. (Eventually, a lot of the assertion
helpers currently in "test_main.c" should probably be organized
into one or more source files of their own.)
Many tests use a loop calling rand() to fill buffers with test data. As
these calls cannot be inlined, this adds up to noticeable overhead:
For example, running on QEMU RISC-V the test_write_format_7zip_large_copy
test took ~22 seconds before and with this change it's ~17 seconds.
This change uses a simpler xorshift64 random number generator that can be
inlined into the loop filling the data buffer. By default the seed for this
RNG is rand(), but it can be overwritten by setting the TEST_RANDOM_SEED
environment variable.
For a native build the difference is much less noticeable, but it's still
measurable: test_write_format_7zip_large_copy takes 314.9 ms ± 3.9 ms
before and 227.8 ms ± 5.8 ms after (i.e. 38% faster for that test).
Recent versions of GNU libc and GCC produce a warning on getcwd(0, PATH_MAX):
test_main.c: In function ‘get_refdir’:
test_main.c:3684:8: error: argument 1 is null but the corresponding size argument 2 value is 4096 [-Werror=nonnull]
3684 | pwd = getcwd(NULL, PATH_MAX);/* Solaris getcwd needs the size. */
This is because getcwd() is marked with the 'write_only (1, 2)' attribute.
Using the alternate getcwd(NULL, 0) path which is supported by GNU libc avoids this.
