not sure why, but msan fires an "unitialized variable" error
when time gets properly initialized by timespec_get().
Maybe in some cases, not all bytes of the structure are initialized ?
Or maybe msan fails to detect the initialization ?
Anyway, pre-initializing the variable before passing it to timespec_get() works.
C11 mandates the definition of timespec_get() and TIME_UTC.
However, FreeBSD11 announce C11 compliance, but does not provifr timespec_get(),
breaking link stage for benchfn.
Since it does not provide TIME_UTC either, which is also required by C11,
test this macro: this will automatically rule out FreeBSD 11 for this code path
(it will use the backup C90 path instead, based on clock_t).
The issue seeems fixed in FreeBSD 12.
benchfn used to rely on mem.h, and util,
which in turn relied on platform.h.
Using benchfn outside of zstd required to bring all these dependencies.
Now, dependency is reduced to timefn only.
This required to create a separate timefn from util,
and rewrite benchfn and timefn to no longer need mem.h.
Separating timefn from util has a wide effect accross the code base,
as usage of time functions is widespread.
A lot of build scripts had to be updated to also include timefn.
While fixing the detection of symbolic links on OpenBSD I noticed
inconsistent behaviour:
$ echo hello > hello
$ ln -s hello world
$ zstd hello world
Warning : world is a symbolic link, ignoring
hello :316.67% ( 6 => 19 bytes, hello.zst
$ ls *.zst
hello.zst
$ zstd world
world :316.67% ( 6 => 19 bytes, world.zst)
$ ls *.zst
hello.zst world.zst
In #1520 it is described that FreeBSD doesn't detect symbolic links. The
same is true for OpenBSD. This diff fixes this issue for OpenBSD. I'm
guessing that something similar works for FreeBSD as well. However, I'm
unable to test this.
was broken by #1505.
I'm surprised it passed CI tests.
LZ4 tests are part of the "Extended" tests on Travis CI,
which are run on "master" and in "cron" jobs.
Since latest cron job did not failed,
especially this one : https://travis-ci.org/facebook/zstd/jobs/484365040
it suggests cron jobs are no longer using `dev` branch.
To be investigated
fseek() doesn't indicate when it moves past the end of a file.
Consequently, if a file is truncated within its last block, the error would't be detected.
This PR adds a test scenario that induces this situation using a small compressed file of only one block in size.
This test is added to tests/playTests.sh
Check is implemented by ensuring that the filehandle position is equal to the filesize upon exit.
Similar to Apple, use the native physical core count sysctl, when available.
This is a little repetitive (it's basically the __APPLE__ method plus the
otherBSD method concatenated together) but seemed clearer than any way that
would totally eliminate repetition.
The __FreeBSD_version check only tests the version of the FreeBSD kernel
that zstd is compiled on; importantly, it may be run on a different version.
So the compile-time check is a little naive and needs to be able to fallback
to work on older versions of FreeBSD. For a similar reason, it may make
sense to simply eliminate the __FreeBSD_version check entirely. The
tradeoff is that a spurious sysctlbyname would be issued when -T0 is used on
older kernels.
due to bad support of inode identifiers.
On Visual, option is limited to same file name,
which is imperfect, but way better than disabling the feature entirely.
It's enough to pass associated tests.
On Windows, the equivalent of `/dev/null` is `NUL`.
When tests are run under msys2/minGW,
the environment identifies itself as Windows,
hence the script uses `NUL` instead of `/dev/null`
but the environment will consider `NUL` to be a regular file name.
Consequently, `NUL` will be overwritten during tests,
triggering an error.
This patch uses flag `-f` to force such overwrite
passing the test.
as suggested in #1441.
generally U32 and unsigned are the same thing,
except when they are not ...
case : 32-bit compilation for MIPS (uint32_t == unsigned long)
A vast majority of transformation consists in transforming U32 into unsigned.
In rare cases, it's the other way around (typically for internal code, such as seeds).
Among a few issues this patches solves :
- some parameters were declared with type `unsigned` in *.h,
but with type `U32` in their implementation *.c .
- some parameters have type unsigned*,
but the caller user a pointer to U32 instead.
These fixes are useful.
However, the bulk of changes is about %u formating,
which requires unsigned type,
but generally receives U32 values instead,
often just for brevity (U32 is shorter than unsigned).
These changes are generally minor, or even annoying.
As a consequence, the amount of code changed is larger than I would expect for such a patch.
Testing is also a pain :
it requires manually modifying `mem.h`,
in order to lie about `U32`
and force it to be an `unsigned long` typically.
On a 64-bit system, this will break the equivalence unsigned == U32.
Unfortunately, it will also break a few static_assert(), controlling structure sizes.
So it also requires modifying `debug.h` to make `static_assert()` a noop.
And then reverting these changes.
So it's inconvenient, and as a consequence,
this property is currently not checked during CI tests.
Therefore, these problems can emerge again in the future.
I wonder if it is worth ensuring proper distinction of U32 != unsigned in CI tests.
It's another restriction for coding, adding more frustration during merge tests,
since most platforms don't need this distinction (hence contributor will not see it),
and while this can matter in theory, the number of platforms impacted seems minimal.
Thoughts ?
Josh Soref
Yann Collet
Ori Peleg
shakeelrao
Nick Terrell
Björn Ketelaars
Peter (Stig) Edwards
Karl Ostmo
Lzu Tao
Conrad Meyer
Yann Collet