[CRIU] [PATCH 1/3] libcriu: use criu_opts structure to keep all the options

[Kuprieiev Ruslan]

On 07/21/2015 12:10 PM, Andrew Vagin wrote:
> On Wed, Jul 15, 2015 at 04:45:02AM +0300, Ruslan Kuprieiev wrote:
>> criu_opts contains rpc options and notify callback,
>> so we can keep all options in just one structure.
>> This will allow us to easily extend libcriu functionality
>> and yet keep all options in one place.
>>
>> We're also not hiding rpc opts structure anymore, so
>> it is pretty clear where power-user should put his own
>> CriuOpts instance if he would like to do that.
>>
>> Signed-off-by: Ruslan Kuprieiev <rkuprieiev at cloudlinux.com>
>> ---
>>   Makefile   |   2 +-
>>   lib/criu.c | 469 ++++++++++++++++++++++++-------------------------------------
>>   lib/criu.h |  81 ++++++-----
>>   3 files changed, 228 insertions(+), 324 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index 6ed4c90..d67ed0b 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -161,7 +161,7 @@ SYSCALL-LIB	:= $(ARCH_DIR)/syscalls.built-in.o
>>   ARCH-LIB	:= $(ARCH_DIR)/crtools.built-in.o
>>   CRIU-SO		:= libcriu
>>   CRIU-LIB	:= lib/$(CRIU-SO).so
>> -CRIU-INC	:= lib/criu.h include/criu-plugin.h include/criu-log.h protobuf/rpc.proto
>> +CRIU-INC	:= lib/criu.h include/criu-plugin.h include/criu-log.h protobuf/rpc.proto protobuf/rpc.pb-c.h
>>   ifeq ($(piegen-y),y)
>>   piegen		:= pie/piegen/piegen
>>   endif
>> diff --git a/lib/criu.c b/lib/criu.c
>> index 329d051..3b36ddd 100644
>> --- a/lib/criu.c
>> +++ b/lib/criu.c
>> @@ -19,8 +19,7 @@
>>   const char *criu_lib_version = CRIU_VERSION;
>>   
>>   static char *service_address = CR_DEFAULT_SERVICE_ADDRESS;
>> -static CriuOpts *global_opts;
>> -static int (*notify)(char *action, criu_notify_arg_t na);
>> +static criu_opts *global_opts;
>>   static int saved_errno;
>>   
>>   void criu_set_service_address(char *path)
>> @@ -31,42 +30,53 @@ void criu_set_service_address(char *path)
>>   		service_address = CR_DEFAULT_SERVICE_ADDRESS;
>>   }
>>   
>> -int criu_local_init_opts(void **o)
>> +int criu_local_init_opts(criu_opts **o)
>>   {
>> -	if (*o) {
>> -		notify = NULL;
>> -		criu_opts__free_unpacked((CriuOpts *)*o, NULL);
>> +	criu_opts *opts = NULL;
>> +	CriuOpts *rpc = NULL;
>> +
>> +	opts = *o;
>> +
>> +	if (opts && opts->rpc)
>> +		criu_opts__free_unpacked(opts->rpc, NULL);
> Do we need to release opts too?

Oops, yes, we do.

>> +
>> +	rpc = malloc(sizeof(CriuOpts));
>> +	if (rpc == NULL) {
>> +		perror("Can't allocate memory for criu RPC opts");
>> +		return -1;
>>   	}
>>   
>> -	*o = malloc(sizeof(CriuOpts));
>> -	if (*o == NULL) {
>> +	criu_opts__init(rpc);
>> +
>> +	opts = malloc(sizeof(criu_opts));
>> +	if (opts == NULL) {
> You leak rpc here
>

Oops again. =)
Thanks, will send incremental fix soon(as this patch is already applied).