Don't call ftruncate() on stdout

[mart-jansink]

While - seems to be explicitly supported as the output option (because if(strcmp(fsout, "-") == 0) is checked), this doesn't work since ftruncate is called on it anyway in copy_file. This commit adds a small check on the given dst, just like it does in the loop that actually writes the output.

[josch]

Interesting. I'm a heavy user of the - output option and never saw this error. What OS are you running?

[mart-jansink]

A form of Linux that's running on the QNAP NAS devices. I'm saying "a form of" because it has some limitations compared to distributions like Ubuntu, e.g. sudo doesn't work because they completely reworked the user management. But I would never had thought that something related to that would be the cause of this issue. My bad, I'll give it a go with Ubuntu to confirm.

Regardless, I will need to run this tool on such QNAP devices and have already tested that this commit fixes genext2fs for it.

[josch]

Oh I wasn't trying to shoot down your contribution. It looks sane to me (without having tested it myself).

[mart-jansink]

No problem, I never took it like that :-) But I can confirm that using the stdout works as expected on a regular Ubuntu, without this commit being necessary. So it appears to be something more obscure, probably related to the version of Linux running on QNAP devices.

I'm no expert in C, so if you think that there's some risk of this commit breaking things I'd completely understand if you feel like this is an edge case that's not worth solving. Applying it as a patch already solved the problem for me, so I'm fine either way.

[bestouff]

@mart-jansink could you try the exact same genext2fs command under strace on your QNAP (without your patch) and a regular Linux, and report the diff ? I'm curious to see what's changing.

[mart-jansink]

While doing this I found that it isn't actually the QNAP Linux version that's causing the problem. I didn't mention that I'm calling genext2fs from a node.js script that takes the stdout, instead of running something like genext2fs -N 128 -m 0 -b 1048576 - > test.ext from the shell. The latter works across the board, while using node.js doesn't on either a QNAP or regular Ubuntu.

The error is easily reproduced by running the following with node.js (at least the LTS one, v14.15.3):

var child_process = require( "child_process" );
var fs = require( "fs" );

var child = child_process.spawn( "genext2fs-patched", [ "-N", 128, "-m", 0, "-b", ( 1024 * 1024 ), "-" ] )
  .on( "error", console.error );

child.stderr
  .on( "data", function( chunk ) { console.log( chunk.toString() ); } );

child.stdout
  .pipe( fs.createWriteStream( "test.ext2" ) )
  .on( "error", console.error );

I can't tell if node.js is doing something special with the stdout of a spawned child process, nor could I find any issues like this for it.

[mart-jansink]

Please ignore the node.js part, its not necessarily relevant either.

The problem appears to be piping the output to another program instead of redirecting it to a file. Under Ubuntu 20.04 genext2fs -N 128 -m 0 -b 1048576 - > test.ext2 works as expected but genext2fs -N 128 -m 0 -b 1048576 - | gzip -c - > test.ext2.gz fails with genext2fs: copy_file: ftruncate: Invalid argument. I've run strace on both examples and attached the output:

• strace_pipe.txt
• strace_redirect.txt

[bestouff]

How does genext2fs manage to work with a piped output ? Do seek() and arbitrary write() work on a pipe ? I'm surprised.

[bestouff]

Anyway in this case the solution is probably just to unconditionally attempt an ftruncate(), without protesting if it fails. So revert your commit, and remove the if and the perror....

[mart-jansink]

I just applied the same logic, for consistency, as it's done a few lines below:

genext2fs/genext2fs.c

Line 2930 in 3b99f4a

if ((dst != stdout) && fs->holes && is_blk_empty(b)) {

This is probably also the answer to your surprise, genext2fs doesn't try to seek() on a piped output because of this.

Do you still like me to make the change?