comm: share standard input between both operands - #14376
Open
harshasiddartha wants to merge 1 commit into
Open
Conversation
`comm - -` locked standard input once per operand and deadlocked on the second lock, before reading anything. Hand both operands the same lock instead, so they take lines from the one stream in turn, which is what GNU comm prints for the same input.
|
GNU testsuite comparison: |
Contributor
|
I don't want to have stdin specific code just for 1 undefined behaviour. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
printf 'test' | comm - -hangs forever without printing anything.Each operand is opened on its own, and
-becomesstdin().lock(). That lock is not reentrant, so the second-blocks on it before any input is read.Both operands now share one lock, taking lines from the single stream in turn. That is what GNU comm prints too: for
a\nb\nc\non standard input, GNU and this branch both writea,\tb,c. GNU also reportscomm: -: Bad file descriptorand exits 1, which comes from it closing standard input twice; that is not reproduced here, and the exit status is 0 as with BSD comm.One difference I did not chase: with
--total, GNU prints no totals line, because it dies in the double close before reaching it, while this branch prints the counts. That follows from the same double-close artifact described above.Tested with a new case in
tests/by-util/test_comm.rsthat pipesa\nb\nc\nintocomm - -and checks the output. It has an explicit timeout, so it fails instead of hanging if the deadlock ever comes back; it does fail on that timeout without this change.Fixes #14219