Re: External checkpoint patch

[Cedric Le Goater <clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>]

minor issue,


> -struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags)
> +static struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags)
>  {
>  	struct cr_ctx *ctx;
> +	int err;
> 
>  	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>  	if (!ctx)
>  		return ERR_PTR(-ENOMEM);

with the err goto, you're calling cr_ctx_free() in a few places where
ctx is partially initialized. you should at least do :

	INIT_LIST_HEAD(&ctx->pgarr_list);

right after the kzalloc() and probably check the value of ctx->hbuf. 
There might be other issues.

> +	ctx->flags = flags;
> +	ctx->pid = pid;
> +
> +	err = -EBADF;
>  	ctx->file = fget(fd);
> -	if (!ctx->file) {
> -		cr_ctx_free(ctx);
> -		return ERR_PTR(-EBADF);
> -	}
> +	if (!ctx->file)
> +		goto err;
>
> 
> +	err = -ENOMEM;
>  	ctx->hbuf = kmalloc(CR_HBUF_TOTAL, GFP_KERNEL);
> -	if (!ctx->hbuf) {
> -		cr_ctx_free(ctx);
> -		return ERR_PTR(-ENOMEM);
> -	}
> +	if (!ctx->hbuf)
> +		goto err;
> 
> -	if (cr_objhash_alloc(ctx) < 0) {
> -		cr_ctx_free(ctx);
> -		return ERR_PTR(-ENOMEM);
> -	}
> +	if (cr_objhash_alloc(ctx) < 0)
> +		goto err;
> 
>  	/*
>  	 * assume checkpointer is in container's root vfs
> @@ -206,12 +206,13 @@ struct cr_ctx *cr_ctx_alloc(pid_t pid, int fd, unsigned long flags)
> 
>  	INIT_LIST_HEAD(&ctx->pgarr_list);
> 
> -	ctx->pid = pid;
> -	ctx->flags = flags;
> -
>  	ctx->crid = atomic_inc_return(&cr_ctx_count);
> 
>  	return ctx;
> +
> + err:
> +	cr_ctx_free(ctx);
> +	return ERR_PTR(err);
>  }
> 
>  /**

here's a patch for it.

C.

Signed-off-by: Cedric Le Goater <clg-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
---
 checkpoint/sys.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: 2.6.27-lxc/checkpoint/sys.c
===================================================================
--- 2.6.27-lxc.orig/checkpoint/sys.c
+++ 2.6.27-lxc/checkpoint/sys.c
@@ -217,7 +217,8 @@ static void cr_ctx_free(struct cr_ctx *c
 	if (ctx->file)
 		fput(ctx->file);
 
-	kfree(ctx->hbuf);
+	if (ctx->hbuf)
+		kfree(ctx->hbuf);
 
 	if (ctx->vfsroot)
 		path_put(ctx->vfsroot);
@@ -239,6 +240,7 @@ static struct cr_ctx *cr_ctx_alloc(pid_t
 	if (!ctx)
 		return ERR_PTR(-ENOMEM);
 
+	INIT_LIST_HEAD(&ctx->pgarr_list);
 	ctx->flags = flags;
 
 	err = -EBADF;
@@ -265,8 +267,6 @@ static struct cr_ctx *cr_ctx_alloc(pid_t
 	ctx->vfsroot = &ctx->root_task->fs->root;
 	path_get(ctx->vfsroot);
 
-	INIT_LIST_HEAD(&ctx->pgarr_list);
-
 	ctx->crid = atomic_inc_return(&cr_ctx_count);
 
 	return ctx;