implement is_running()

[hendrikmuhs]

Implements a is_running() method to check whether a process is running. For some usecases this makes the observer thread unnecessary.

For waitpid(unix) I had to cache the exit code, IMHO now it behave also more consistent as get_exit_status can be called multiple times. For windows it should work without this trick.

Windows implementation should work but needs to be tested.

[eidheim]

Great, thank you. A couple of thoughts before I study the changes more:

• Instead of is_running(), could the function maybe be something like try_get_exit_status() that is non-blocking?
• the exit_status type is DWORD on Windows (32-bit unsigned integer). I think that should be taken into consideration, that is, use type int on unix-like systems, and type DWORD on Windows.

[hendrikmuhs]

For a try_get_exit_status() (or a parameter blocking=true/false for get_exit_status())

we would need a magic value as return value as I need to distinguish between an exit code and the 'still running' condition. As exit codes can be anything we would need to take a uncommon value and hope no one is using it.

[foonathan]

Or you can use something like std::optional. But given that it is only in std since C++17 it is probably not a good idea to have an external dependency just for that.

[eidheim]

I think we should keep this a c++11 library a few more years, but I agree that std::optional should be used sometime in the future.

Presently, I guess the best solution would be a declaration like this: bool try_get_exit_status(exit_status_type &exit_status) const noexcept. Note also that I added const since I think at least initially that the exit_status should not be stored in a class variable.

An alternative declaration is: std::pair<bool, exit_status_type> try_get_exit_status() const noexcept, but since the returned exit_status_type value is not always valid, I think the this is a less good alternative.

[eidheim]

Also, one of the reasons I prefer try_get_exit_status() instead of is_running() is that try_get_exit_status() is more atomic, and one might possibly take advantage of this, than running two functions sequentially like if(process.is_running()) exit_status = process.get_exit_status();. This is better I think: if(process.try_get_exit_status(exit_status)) // do something

[hendrikmuhs]

Let's keep it simple! An out parameter will do.

For me, is_running() made sense in my application, but try_get_exit_status(...) will do the same job.

Ok, I will change it accordingly.

[eidheim]

Oh, I see the current get_exit_status returns int. Can probably still use int instead of exit_status_type that I mentioned above. Just make sure to static_cast in the windows specific code after receiving the value from the windows API.

edit: fixed ``s

[eidheim]

And the function cannot be const I see now, since one might close file descriptors.

[eidheim]

Thank you again, the commits have been pushed to master. I did some minor changes here: https://github.com/eidheim/tiny-process-library/commit/2984e8d72e07817291c4db8fd1b04e7739219af5. I'll test it on Windows during the weekend I hope.