Thread overview | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
March 10, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 Jon Degenhardt <jrdemail2000-dlang@yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|ProcessPipes (std.process) |ProcessPipes (std.process) |should provide a test for a |should provide a test for a |null pid struct |null pid -- |
March 10, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 Jon Degenhardt <jrdemail2000-dlang@yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Priority|P1 |P2 --- Comment #1 from Jon Degenhardt <jrdemail2000-dlang@yahoo.com> --- The ProcessPipes struct provides a method, pid(), that returns the Pid of the attached process. The pid() method asserts if the private member is null: @property Pid pid() @safe nothrow { assert(_pid !is null); return _pid; } The problem is that there is no method available to determine if pid is null prior to call the pid() method. In idiomatic use this unlikely to be an issue, as entering a block requiring access to the pid is typically conditioned on successful creation of the pid. An example from the doc: auto pipes = pipeProcess("my_application", Redirect.stdout | Redirect.stderr); scope(exit) wait(pipes.pid); ... more code ... In the above, the scope exit block is only entered if pid is successfully created. However, if process creation is deferred, the pid might never be assigned. E.g. ProcessPipes pipes; scope(exit) wait(pipes.pid); ... code ... pipes = pipeProcess("my_application", Redirect.stdout | Redirect.stderr); ... code ... The above will fail in the 'wait(pipes.pid)' call if the _pid member is null. What seems desired is a way to write something equivalent to: scope(exit) if (!pipes.isNullPid) wait(pipes.pid) -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 RazvanN <razvan.nitu1305@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |razvan.nitu1305@gmail.com --- Comment #2 from RazvanN <razvan.nitu1305@gmail.com> --- I think the solution here is to remove the assert and expect the user to check if the result of pid is null. -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 --- Comment #3 from RazvanN <razvan.nitu1305@gmail.com> --- PR: https://github.com/dlang/phobos/pull/5621 -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 RazvanN <razvan.nitu1305@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |pull -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 --- Comment #4 from Vladimir Panteleev <dlang-bugzilla@thecybershadow.net> --- (In reply to Jon Degenhardt from comment #1) > The problem is that there is no method available to determine if pid is null prior to call the pid() method. Why not: if (pipes !is ProcessPipes.init) (In reply to RazvanN from comment #2) > I think the solution here is to remove the assert and expect the user to check if the result of pid is null. I think that's OK too, though it has the small downside that null dereferencing bugs are harder to diagnose on non-Windows systems. We could also add an implicit conversion to bool. -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 Vladimir Panteleev <dlang-bugzilla@thecybershadow.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Hardware|x86 |All OS|Mac OS X |All -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 --- Comment #5 from Jon Degenhardt <jrdemail2000-dlang@yahoo.com> --- (In reply to Vladimir Panteleev from comment #4) > (In reply to Jon Degenhardt from comment #1) > > The problem is that there is no method available to determine if pid is null prior to call the pid() method. > > Why not: if (pipes !is ProcessPipes.init) Isn't 'is' a compile-time expression? The test needs to be run-time. Though that would mean 'if (pipes != ProcessPipes.init)' would work. Either way, these seem like very obscure ways to do such a test. > > (In reply to RazvanN from comment #2) > > I think the solution here is to remove the assert and expect the user to check if the result of pid is null. > > I think that's OK too, though it has the small downside that null dereferencing bugs are harder to diagnose on non-Windows systems. > > We could also add an implicit conversion to bool. Implicit conversion to bool sounds pretty good. Then it'd become: scope(exit) if (pipes) wait(pipes.pid) For me the downside of simply removing the assert and returning a null Pid object is that it uses an internal implementation detail of ProcessPipes to describe the state of the ProcessPipes instance. Modularity is better served by separating the ProcessPipes state from the state internal _pid member, at least in the ProcessPipes API. (This argument also says the original suggested API name of 'isNullPid' is not a good name.) -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 --- Comment #6 from Jon Degenhardt <jrdemail2000-dlang@yahoo.com> --- (In reply to Jon Degenhardt from comment #5) > (In reply to Vladimir Panteleev from comment #4) > > (In reply to Jon Degenhardt from comment #1) > > > The problem is that there is no method available to determine if pid is null prior to call the pid() method. > > > > Why not: if (pipes !is ProcessPipes.init) > > Isn't 'is' a compile-time expression? The test needs to be run-time. Though that would mean 'if (pipes != ProcessPipes.init)' would work. Either way, these seem like very obscure ways to do such a test. Never mind the 'compile-time expression' part, 'is' is a run-time test. Still, it seems very obscure, and it's not obvious that a ProcessPipes struct that has a null PID due to process startup failure will be in the same state as the struct at .init time. -- |
July 17, 2017 [Issue 17250] ProcessPipes (std.process) should provide a test for a null pid | ||||
---|---|---|---|---|
| ||||
https://issues.dlang.org/show_bug.cgi?id=17250 --- Comment #7 from github-bugzilla@puremagic.com --- Commits pushed to master at https://github.com/dlang/phobos https://github.com/dlang/phobos/commit/b4284db1f937e90fec6ffe4a9e4e2bf4783799bb Fix Issue 17250 - ProcessPipes (std.process) should provide a test for a null pid https://github.com/dlang/phobos/commit/1e296c11d8f8083402f8611d495cdcd015ff6fb3 Merge pull request #5621 from RazvanN7/Issue_17250 Fix Issue 17250 - ProcessPipes (std.process) should provide a test for a null pid merged-on-behalf-of: Vladimir Panteleev <github@thecybershadow.net> -- |
Copyright © 1999-2021 by the D Language Foundation