Daniel P. Berrange <berra...@redhat.com> wrote:
> On Fri, Mar 16, 2018 at 05:49:07PM +0000, Daniel P. Berrangé wrote:
>> On Fri, Mar 16, 2018 at 12:53:49PM +0100, Juan Quintela wrote:
>> > Signed-off-by: Juan Quintela <quint...@redhat.com>
>> > ---
>> > migration/ram.c | 20 ++++++++++++++++++++
>> > 1 file changed, 20 insertions(+)
>> >
>> > diff --git a/migration/ram.c b/migration/ram.c
>> > index 7266351fd0..1b8095a358 100644
>> > --- a/migration/ram.c
>> > +++ b/migration/ram.c
>> > @@ -414,6 +414,16 @@ static void terminate_multifd_send_threads(Error
>> > *errp)
>> > {
>> > int i;
>> >
>> > + if (errp) {
>> > + MigrationState *s = migrate_get_current();
>> > + migrate_set_error(s, errp);
>>
>> This doesn't look quiet right. You're checking if 'errp' is a non-NULL,
>> which just tells you if the caller wants to collect the error, not
>> whether an error has happened. For the latter you need
>>
>> if (errp && *errp)
>>
>> seems a little strange though for the caller to pass an error into this
>> method for reporting.
>
> Oh wait, I'm being mislead by the unusual parameter name.
>
> An "errp" name should only ever be used for a "Error **", but we
> only have an "Error *" here.
Copy & Paste O:-)
> So just fix the parameter name to be "err" instead of "errp".
Done.
Thanks,
