Subject: [boost] [Process] List of small issues
From: Artyom (artyomtnk_at_[hidden])
Date: 2011-01-13 09:35:53
I've made a micro review of Boost.Process, there are small
issues I've found:
- stream buffer implementation:
you need to check the errno if it is EINTR,
returning -1 and getting EINTR is not a problem and thus
it should retry.
Same as in underflow() - check for EINTR and retry, but also
there is a problem that you do not check that you had fully
written the data.
For example what I write out << std::flush sync() is called
and I expect the all data should be written to device, so
if return value is less then the size of the buffer you should
retry and write again till buffer is empty, you get error or EOF.
- Windows and Unicode.
You are using CreateProcessA. I would recommend to always use
wide API and convert narrow strings to wide similarly to what
boost::filesystem::v3 does, so for example where the global
locale as utf-8 facet you would convert narrow strings to wide
and run it.
1. You can also always assume that strings under windows are UTF-8
and always convert them to wide string before system calls.
This is I think better approach, but it is different from what
most of boost does.
2. I do not recommend adding wide API - makes the code much uglier,
rather convert normal strigns to wide strings before system call.
- It may be very good addition to implement full support of putback.
I've noticed that you planned asynchronous notification and so on
but I think it is quite important to add feature that provide
an ability to wait for multiple processes to terminate
and have timeout.
It can be done using sigtimedwait/sigwait and assigned signal handlers
P.S.: Good luck with the review library looks overall very nice.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk