Eric Blake <ebl...@redhat.com> writes: > On 08/29/2018 08:57 PM, John Snow wrote: >> Jobs presently use both an Error object in the case of the create job, >> and char strings in the case of generic errors elsewhere. >> >> Unify the two paths as just j->err, and remove the extra argument from >> job_completed. The integer error code for job_completed is kept for now, >> to be removed shortly in a separate patch. >> >> Signed-off-by: John Snow <js...@redhat.com> >> --- > >> +++ b/job.c > >> @@ -666,8 +666,8 @@ static void job_update_rc(Job *job) >> job->ret = -ECANCELED; >> } >> if (job->ret) { >> - if (!job->error) { >> - job->error = g_strdup(strerror(-job->ret)); >> + if (!job->err) { >> + error_setg(&job->err, "%s", g_strdup(strerror(-job->ret))); > > Memleak. Drop the g_strdup(), and just directly pass strerror() > results to error_setg(). (I guess we can't quite use > error_setg_errno() unless we add additional text beyond the strerror() > results).
Adding such text might well be an improvement. I'm not telling you to do so (not having looked at the context myself), just to think about it. > With that fixed, > Reviewed-by: Eric Blake <ebl...@redhat.com>