closing stdin on pipe

[andreas-ibm]

May I request that someone checks their standard unix file descriptor numbers... from replay.c line 128:

    /* close stdin if reading from it (needed for some OS's) */
    if (strncmp(path, "-", 1) == 0)
        close(1);

(HINT: stdin is 0, 1 is stdout...)

[fklassen]

Check for what?

[andreas-ibm]

I was being sarcastic.

the block of code either doesn't do what's commented, or the comment is wrong.

The comment says "close stdin", the code closes stdout.

This has the effect of dropping all output from the command if you happen to be pipeing data to it via stdin. I've removed the block of code in my copy and it works much better.

I didn't simply send a PR, because I couldn't figure out the intent behind the code, I have a sneaky feeling that it's an attempted bugfix that might have inadvertently worked on "some OS" without the author of the code realising the unintended side effect. Changing the code to close(0) which would do as the comment says, but I believe would have very detrimental effects.

[fklassen]

Ahh ... I was looking at the code, not the comment. That code is well before my time, so I may have to guess at the intent. I'll schedule comment update for a future release.

[andreas-ibm]

Could I suggest removing the whole block of code please? Closing stdout causes errors down the line when it tries to write (e.g. statistics) to stdout.

[fklassen]

I'll review and test. We had some issues come up with stdin/stdout not being closed. I just want to make sure we are not introducing any regression.

[fklassen]

See #416 and #470. Not a bug.

[andreas-ibm]

I'd still argue it is a bug.
#416 introduced this change:

    /* avoid making STDIN non-blocking */
    int nb = 0;
    ioctl(0, FIONBIO, &nb)

Which indeed sets it to blocking mode using the old Unix way, but I fail to see how it relates to the current code:

    /* close stdin if reading from it (needed for some OS's) */
    if (strncmp(path, "-", 1) == 0)
        close(1);

a) I've never heard of an OS that needs you to close STDIN when reading from STDIN!
b) you're closing STDOUT

I honestly don't see the connection between #416, #470 and #479. #416 was a change to avoid non-blocking, that code is no-longer present in the codebase. #470 is asking that you set blocking mode prior to termination. Whilst #479 is merely a request that you stop closing STDOUT!

I don't use tcpreplay anymore as I've fixed my original problem in a different way, I'd still expect users of the code to be surprised when they don't get any output when passing it data on STDIN.

[fklassen]

Appears I got STDIN/STDOUT confused. Reopening.

[fklassen]

fixed in #510