Sparse file support breaks pipe output on decompression.

[eborisch]

On FreeBSD; 'liblz4-129'; installed via portmaster.

This used to work, but fails with 129:

cat /dev/urandom | lz4c | lz4c -d | cat > /dev/null
Error 68 : Skip error (sparse file)

I can make the piped output version work again with --no-sparse on the decompression step; i.e. both of these work:

cat /dev/urandom | lz4c | lz4c -d --no-sparse | cat > /dev/null
cat /dev/urandom | lz4c | lz4c -d > /dev/null

It seems to me that --no-sparse should be the default when outputting to a pipe; and to be compatible with other (bzip2, gzip, etc) filters that don't require an additional flag. This could be accomplished either via stat() and S_ISREG() testing, or by gracefully recovering on a skip error. (My guess is the former will be more straightforward to implement, and the latter a more robust solution for unexpected cases.)

[t-mat]

Hi, @eborisch. Thanks for the report !

Recently, we have same reports

• Disabled sparse file mode when output file is terminal. #105 : Disabled sparse file mode when output file is terminal.
• Sparse file by default is breaking pipelines @ LZ4c

And it has been resolved by 58b5aad.

So, could you try latest dev branch ?
The following command will work on BSD platforms:

cd
mkdir my-lz4 && cd my-lz4
git clone -b dev https://github.com/Cyan4973/lz4.git .
cd programs
make lz4c
# Or use "gmake" :
# gmake lz4c
cat /dev/urandom | ./lz4c | ./lz4c -d | cat > /dev/null

EDIT : Fix wrong commit hash.

[t-mat]

note : xz has interesting comment for sparse file mode
"Creating a sparse file is not possible when O_APPEND is active".

[eborisch]

So sorry for the duplicate -- I thought I had searched, but clearly I missed it.

Confirmed that latest dev branch works as advertised on FreeBSD 10.1. Closing. Thanks for the excellent SW & quick response! I'll contact the FreeBSD port maintainer to make sure they are aware. Any chance of a new tagged release with this fix?

[t-mat]

No problem 😄 I also need more smart search for issues on github. Anyway, thanks for confirming !
For new release, since three consecutive issues are unusual for this project, I'm asking @Cyan4973 for now.

[Cyan4973]

Thanks for reporting.
The current patch in "dev" branch works well enough for lz4cat situations,
unfortunately, the new test case reported by @t-mat, using >> redirector,
would silently fail, meaning there is not even a skip error report (!) .
Getting around this problem seems to require some OS-specific libraries.

I'm therefore inclined on selecting the stronger option to disable sparse support on stdout.

For your comments

[ido]

FWIW, selfishly, I'd prefer if you kept/fixed sparse support on all output types on OSes that support it well if you can. Or, at the very least, on Linux... :-/

[Cyan4973]

The short term proposal
would be to have "default sparse support disabled on stdout"
but it could still be enabled by using --sparse command, which overrides defaults settings.

Default sparse support would remain enabled for direct write-to-file operations, which don't suffer above issues.

Is that nonetheless a problem for you @ido ?

[eborisch]

Seems like a reasonable approach. The associated error message could also call out the fix. "Try --no-sparse."

Corner cases (named pipe passed as output file argument?) likely still exist. To be truly robust, but still support sparse files, perhaps the seek on the output stream should be attempted and then gracefully handled (by disabling sparse) on EPIPE / SIGPIPE. Likely only needs to be tested at start of execution; perhaps during argument parsing.

[Cyan4973]

perhaps the seek on the output stream should be attempted and then gracefully handled (by disabling sparse)

This is how it is currently handled within "dev" branch.
The problem is, with >> redirection, fseek() pretends that everything works properly, so there is no way to notice any error.
The >> redirection issue, related to append mode, is only possible in combination with stdout.

The associated error message could also call out the fix. "Try --no-sparse."

Good point !

[Cyan4973]

Latest update in "dev" branch acae59a
fixes >> redirection issue (and already reported console & cat issues)
by disabling sparse mode by default on stdout.
It's still possible to force it using --sparse.

One of the last remaining questions is : should it be made a hotfix (due to lz4cat being currently broken), or can it wait for a future r130 ?

[t-mat]

👍 for hotfix !

note : I've reopened this issue, for @Cyan4973's commit and future PR.

[Cyan4973]

Moved to master