[Libguestfs] [PATCH 2/3] NEW API: add a new api e2fsck

[Richard W.M. Jones]

On Fri, Jan 13, 2012 at 10:55:57PM +0800, Wanlong Gao wrote:
> 
> From: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> 
> Add a new api e2fsck with two options:
> correct: same as '-p' option of e2fsck
> forceall: same as '-y' option of e2fsck

API looks good in general.  A few comments below.

> Thanks for Rich's idea.
> 
> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/ext2.c                  |   42 ++++++++++++++++++++++++++++++++++++++++
>  generator/generator_actions.ml |   24 ++++++++++++++++++++++
>  src/MAX_PROC_NR                |    2 +-
>  3 files changed, 67 insertions(+), 1 deletions(-)
> 
> diff --git a/daemon/ext2.c b/daemon/ext2.c
> index c280ca2..b0dc6da 100644
> --- a/daemon/ext2.c
> +++ b/daemon/ext2.c
> @@ -294,6 +294,48 @@ do_resize2fs_M (const char *device)
>  }
>  
>  int
> +do_e2fsck (const char *device,
> +           int correct,
> +           int forceall)
> +{
> +  const char *argv[MAX_ARGS];
> +  char *err;
> +  size_t i = 0;
> +  int r;
> +  char prog[] = "e2fsck";
> +
> +  if (e2prog (prog) == -1)
> +    return -1;
> +
> +  if (correct && forceall) {
> +    reply_with_error("%s", "Only one of the options may be specified");

You don't need "%s", ... here.

The reason we use "%s", err later on is because err comes from an
external program and may contain %-sequences.  We don't want it to be
interpreted as a format string.  This is never a problem with the
constant string above.

Rest of the patch looks fine, so ACK with that change made.

Anyone else wanna review?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/