Bug #6593
closed
[SDKs] arv-get should use the stdout FD directly, rather than opening /dev/stdout
Added by Brett Smith about 9 years ago.
Updated about 9 years ago.
Description
A process can always write to fd 1, but it may not always be able to open /dev/stdout
. In particular, if a user in an SSH session uses sudo
to open a shell for a different non-root user, /dev/stdout
will be a link to the user's original tty, which the new user probably won't be able to open.
The destination argument should default to None. The main body should catch that case and use sys.stdout as the output file in that case, rather than opening /dev/stdout
. Update other checks as needed.
- Target version changed from 2015-08-19 sprint to 2015-08-05 sprint
- Assigned To set to Peter Amstutz
- Status changed from New to In Progress
Brett Smith wrote:
Reviewing ce12890
- Please update where the command line option help strings refer to /dev/stdout. I think it would be fine to just change them to refer to plain "stdout."
Fixed.
- Under the comment "# Turn on --progress by default", there's a condition
args.destination != '/dev/stdout'
that no longer works as intended.
Thanks for catching that, fixed.
- This bug predates your branch, but I noticed it while reading: this block:
[...]
would better avoid race conditions and other corner cases written as:
[...]
(The old code might unlink the wrong file if the ^C comes in between reading a new outfilename and opening it.)
Done.
Thanks!
Peter Amstutz wrote:
Brett Smith wrote:
- This bug predates your branch, but I noticed it while reading: this block:
[...]
would better avoid race conditions and other corner cases written as:
[...]
(The old code might unlink the wrong file if the ^C comes in between reading a new outfilename and opening it.)
Done.
Sorry, but, uh, you did not copy and paste this correctly.
- The unlink line should change its argument from
outfilename
to outfile.name
.
outfile.closed
is a boolean attribute, not a method. The condition currently in your branch will raise a TypeError. Remove the parentheses.
The branch is good to merge with these changes. Thanks.
- Status changed from In Progress to Resolved
Applied in changeset arvados|commit:66af20886def83f6a20cc1e6587de00cbf2f8b59.
Also available in: Atom
PDF