Eric Blake <ebl...@redhat.com> wrote:
D> On 09/06/2017 06:51 AM, Juan Quintela wrote:
>> So far, we had to free the error after each caller, so just do it
>> here. Once there, tls.c was leaking the error.
>
> You mention tls.c,
>
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> ---
>> migration/channel.c | 1 -
>> migration/migration.c | 10 ++++------
>> migration/migration.h | 4 ++--
>> migration/socket.c | 1 -
>> 4 files changed, 6 insertions(+), 10 deletions(-)
>
> but don't touch it. Am I missing something?
>
It was missing error_free(); So it leaked the Error * variable.
I will improve the message for next version.
>>
>> -void migrate_fd_error(MigrationState *s, const Error *error)
>> +void migrate_fd_error(MigrationState *s, Error *error)
>> {
>
> No comments at definition,
We free it inside now, so it can't be const.
>> +++ b/migration/migration.h
>> @@ -163,8 +163,8 @@ bool migration_has_all_channels(void);
>>
>> uint64_t migrate_max_downtime(void);
>>
>> -void migrate_set_error(MigrationState *s, const Error *error);
>> -void migrate_fd_error(MigrationState *s, const Error *error);
>> +void migrate_set_error(MigrationState *s, Error *error);
>> +void migrate_fd_error(MigrationState *s, Error *error);
>
> or at declaration. That would be worth adding at some point, but this
> patch isn't making it worse.
will add them, thanks.
> The code looks okay in isolation, so if it is only the commit message
> that needs fixing,
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks.
