| Message ID | 7e0c85d46907b6ff3299c5ded3238f590f2c5946.1461609759.git.crobinso@redhat.com |
|---|---|
| State | Accepted |
| Commit | c0e870376c09c67809d899240ac8347928685ce2 |
| Headers | show |
On 05/02/2016 04:48 AM, Michal Privoznik wrote: > On 25.04.2016 20:46, Cole Robinson wrote: >> Let's us de-nest some of the logic, and will simplify upcoming >> patches >> --- >> src/fdstream.c | 73 +++++++++++++++++++++++++++++++++------------------------- >> 1 file changed, 42 insertions(+), 31 deletions(-) >> >> diff --git a/src/fdstream.c b/src/fdstream.c >> index a6a0fbe..681b90e 100644 >> --- a/src/fdstream.c >> +++ b/src/fdstream.c >> @@ -240,6 +240,46 @@ virFDStreamAddCallback(virStreamPtr st, >> return ret; >> } >> >> +static int >> +virFDStreamCloseCommand(struct virFDStreamData *fdst) >> +{ >> + char buf[1024]; >> + ssize_t len; >> + int status; >> + int ret = -1; >> + >> + if (!fdst->cmd) >> + return 0; >> + >> + if ((len = saferead(fdst->errfd, buf, sizeof(buf)-1)) < 0) >> + buf[0] = '\0'; >> + else >> + buf[len] = '\0'; >> + >> + virCommandRawStatus(fdst->cmd); >> + if (virCommandWait(fdst->cmd, &status) < 0) >> + goto error; >> + >> + if (status != 0) { >> + if (buf[0] != '\0') { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", buf); >> + } else if (WIFEXITED(status)) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, >> + _("I/O helper exited with status %d"), >> + WEXITSTATUS(status)); >> + } else { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >> + _("I/O helper exited abnormally")); >> + } >> + goto error; >> + } >> + >> + ret = 0; >> + error: > > Just a small nit, we tend to name 'cleanup' labels that are used in both > successful and unsuccessful return paths. So this should be 'cleanup' to > follow our style. > Thanks for the review. I fixed this and pushed the series now - Cole -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/src/fdstream.c b/src/fdstream.c index a6a0fbe..681b90e 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -240,6 +240,46 @@ virFDStreamAddCallback(virStreamPtr st, return ret; } +static int +virFDStreamCloseCommand(struct virFDStreamData *fdst) +{ + char buf[1024]; + ssize_t len; + int status; + int ret = -1; + + if (!fdst->cmd) + return 0; + + if ((len = saferead(fdst->errfd, buf, sizeof(buf)-1)) < 0) + buf[0] = '\0'; + else + buf[len] = '\0'; + + virCommandRawStatus(fdst->cmd); + if (virCommandWait(fdst->cmd, &status) < 0) + goto error; + + if (status != 0) { + if (buf[0] != '\0') { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", buf); + } else if (WIFEXITED(status)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("I/O helper exited with status %d"), + WEXITSTATUS(status)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("I/O helper exited abnormally")); + } + goto error; + } + + ret = 0; + error: + virCommandFree(fdst->cmd); + fdst->cmd = NULL; + return ret; +} static int virFDStreamCloseInt(virStreamPtr st, bool streamAbort) @@ -285,37 +325,8 @@ virFDStreamCloseInt(virStreamPtr st, bool streamAbort) /* mutex locked */ ret = VIR_CLOSE(fdst->fd); - if (fdst->cmd) { - char buf[1024]; - ssize_t len; - int status; - if ((len = saferead(fdst->errfd, buf, sizeof(buf)-1)) < 0) - buf[0] = '\0'; - else - buf[len] = '\0'; - - virCommandRawStatus(fdst->cmd); - if (virCommandWait(fdst->cmd, &status) < 0) { - ret = -1; - } else if (status != 0) { - if (buf[0] == '\0') { - if (WIFEXITED(status)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("I/O helper exited with status %d"), - WEXITSTATUS(status)); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("I/O helper exited abnormally")); - } - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - buf); - } - ret = -1; - } - virCommandFree(fdst->cmd); - fdst->cmd = NULL; - } + if (virFDStreamCloseCommand(fdst) < 0) + ret = -1; if (VIR_CLOSE(fdst->errfd) < 0) VIR_DEBUG("ignoring failed close on fd %d", fdst->errfd);