On 03.01.2012 20:00, Eric Blake wrote:
On 01/03/2012 10:58 AM, Michal Privoznik wrote:
> It is a good practise to set revents to zero before doing any poll().
> Moreover, we should check if event we waited for really occurred or
> if any of fds we were polling on didn't encountered hangup.
Looks like this also solves:
https://bugzilla.redhat.com/show_bug.cgi?id=770788
> ---
> src/util/command.c | 21 +++++++++++++++++----
> 1 files changed, 17 insertions(+), 4 deletions(-)
>
Should we also be probing for POLLERR? (POLLNVAL shouldn't ever
be
present, because it would indicate a bigger bug in our code).
ACK - what you have is a strict improvement, even if we further decide
to check for POLLERR.
Thanks, pushed with this squashed in:
diff --git a/src/util/command.c b/src/util/command.c
index 9b553f0..bdaa88b 100644
--- a/src/util/command.c
+++ b/src/util/command.c
@@ -1687,7 +1687,10 @@ virCommandProcessIO(virCommandPtr cmd)
memcpy(*buf + *len, data, done);
*len += done;
}
- } else if (fds[i].revents & POLLOUT) {
+ }
+
+ if (fds[i].revents & POLLOUT &&
+ fds[i].fd == infd) {
int done;
/* Coverity 5.3.0 can't see that we only get here if
@@ -1711,7 +1714,9 @@ virCommandProcessIO(virCommandPtr cmd)
VIR_DEBUG("ignoring failed close on fd %d",
tmpfd);
}
}
- } else if (fds[i].revents & POLLHUP) {
+ }
+
+ if (fds[i].revents & (POLLHUP | POLLERR)) {
if (fds[i].fd == errfd) {
VIR_DEBUG("hangup on stderr");
errfd = -1;