As the comment in qapi/error, dereferencing @errp requires ERRP_GUARD(): * = Why, when and how to use ERRP_GUARD() = * * Without ERRP_GUARD(), use of the @errp parameter is restricted: * - It must not be dereferenced, because it may be null. ... * ERRP_GUARD() lifts these restrictions. * * To use ERRP_GUARD(), add it right at the beginning of the function. * @errp can then be used without worrying about the argument being * NULL or &error_fatal. * * Using it when it's not needed is safe, but please avoid cluttering * the source with useless code.
Though currently ga_run_command() only gets &local_err instead of NULL @errp, it's still better to follow the requirement to add the ERRP_GUARD(). But as error.h suggested, the best practice for callee is to return something to indicate success / failure. So make ga_wait_child() return boolean and check the returned boolean in ga_run_command() instead of dereferencing @errp, which eliminates the need of ERRP_GUARD(). Cc: Michael Roth <michael.r...@amd.com> Cc: Konstantin Kostiuk <kkost...@redhat.com> Signed-off-by: Zhao Liu <zhao1....@intel.com> --- qga/commands-posix.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 7f05996495a2..64bb0be94479 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -59,7 +59,7 @@ #endif #endif -static void ga_wait_child(pid_t pid, int *status, Error **errp) +static bool ga_wait_child(pid_t pid, int *status, Error **errp) { pid_t rpid; @@ -70,10 +70,11 @@ static void ga_wait_child(pid_t pid, int *status, Error **errp) if (rpid == -1) { error_setg_errno(errp, errno, "failed to wait for child (pid: %d)", pid); - return; + return false; } g_assert(rpid == pid); + return true; } static ssize_t ga_pipe_read_str(int fd[2], char **str) @@ -178,8 +179,7 @@ static int ga_run_command(const char *argv[], const char *in_str, goto out; } - ga_wait_child(pid, &status, errp); - if (*errp) { + if (!ga_wait_child(pid, &status, errp)) { goto out; } -- 2.34.1