Libguestfs - Dec 2011 - [PATCH] blkid: split the RHEL5 which can't support some options

RHEL5 shoult not support '-p', '-i' and '-o export'
options.
But we just split it according to the '-p' option.

Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
---
 daemon/blkid.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/daemon/blkid.c b/daemon/blkid.c
index a7fd6bc..8c2cdc6 100644
--- a/daemon/blkid.c
+++ b/daemon/blkid.c
@@ -84,18 +84,42 @@ do_vfs_uuid (const char *device)
   return get_blkid_tag (device, "UUID");
 }
 
-char **
-do_blkid(const char *device)
+/* RHEL5 blkid doesn't have the -p(partition info) option and the
+ * -i(I/O limits) option so we must test for these options the first
+ * time the function is called.
+ */
+static int
+test_blkid_p_opt(void)
+{
+  static int result = -1;
+  char *err = NULL;
+
+  int r = commandr (NULL, &err, "blkid", "-p",
"/dev/null", NULL);
+  if (r == -1) {
+    reply_with_error("Could not run 'blkid' command");
+    free(err);
+    return -1;
+  }
+
+  if (err && strstr(err, "invalid option --"))
+    result = 0;
+  else
+    result = 1;
+  free(err);
+  return result;
+}
+
+static char **
+blkid_with_p_opt(const char *device)
 {
   int r;
   char *out = NULL, *err = NULL;
   char **lines = NULL;
-
   char **ret = NULL;
   int size = 0, alloc = 0;
 
-  const char *blkid[] = {"blkid", "-p", "-i",
"-o", "export", device, NULL};
-  r = commandv(&out, &err, blkid);
+  r = command(&out, &err, "blkid", "-c",
"/dev/null",
+               "-p", "-i", "-o",
"export", device, NULL);
   if (r == -1) {
     reply_with_error("%s", err);
     goto error;
@@ -142,12 +166,12 @@ do_blkid(const char *device)
     }
   }
 
+  if (add_string(&ret, &size, &alloc, NULL) == -1) goto error;
+
   free(out);
   free(err);
   free(lines);
 
-  if (add_string(&ret, &size, &alloc, NULL) == -1) return NULL;
-
   return ret;
 
 error:
@@ -158,3 +182,45 @@ error:
 
   return NULL;
 }
+
+static char **
+blkid_without_p_opt(const char *device)
+{
+  char **ret = NULL;
+  int size = 0, alloc = 0;
+
+  if (add_string(&ret, &size, &alloc, "TYPE") == -1) goto
error;
+  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"TYPE")) == -1)
+    goto error;
+  if (add_string(&ret, &size, &alloc, "LABEL") == -1)
goto error;
+  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"LABEL")) == -1)
+    goto error;
+  if (add_string(&ret, &size, &alloc, "UUID") == -1) goto
error;
+  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"UUID")) == -1)
+    goto error;
+  if (add_string_nodup(&ret, &size, &alloc, NULL) == -1) goto
error;
+
+  return ret;
+error:
+  if (ret) free_strings(ret);
+  return NULL;
+}
+
+char **
+do_blkid(const char *device)
+{
+  int r;
+  char *out = NULL, *err = NULL;
+  char **lines = NULL;
+
+  char **ret = NULL;
+  int size = 0, alloc = 0;
+  int blkid_has_p_opt = -1;
+
+  if ((blkid_has_p_opt = test_blkid_p_opt()) == -1)
+    return NULL;
+  else if (blkid_has_p_opt)
+    return blkid_with_p_opt(device);
+  else
+    return blkid_without_p_opt(device);
+}
-- 
1.7.8.rc4

On Mon, Dec 05, 2011 at 02:22:43PM +0800, Wanlong Gao
wrote:
> RHEL5 shoult not support '-p', '-i' and '-o export'
options.
> But we just split it according to the '-p' option.
> 
> Signed-off-by: Wanlong Gao <gaowanlong at cn.fujitsu.com>
> ---
>  daemon/blkid.c |   80
+++++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 files changed, 73 insertions(+), 7 deletions(-)
> 
> diff --git a/daemon/blkid.c b/daemon/blkid.c
> index a7fd6bc..8c2cdc6 100644
> --- a/daemon/blkid.c
> +++ b/daemon/blkid.c
> @@ -84,18 +84,42 @@ do_vfs_uuid (const char *device)
>    return get_blkid_tag (device, "UUID");
>  }
>  
> -char **
> -do_blkid(const char *device)
> +/* RHEL5 blkid doesn't have the -p(partition info) option and the
> + * -i(I/O limits) option so we must test for these options the first
> + * time the function is called.
> + */
> +static int
> +test_blkid_p_opt(void)
> +{
> +  static int result = -1;
Don't initialize 'result' as it prevents the compiler from warning
about problems in the code.
> +  char *err = NULL;
> +
> +  int r = commandr (NULL, &err, "blkid", "-p",
"/dev/null", NULL);
> +  if (r == -1) {
> +    reply_with_error("Could not run 'blkid' command");
  reply_with_error("could not run 'blkid' command: %s", err);
> +    free(err);
> +    return -1;
> +  }
> +
> +  if (err && strstr(err, "invalid option --"))
err is never NULL here, since commandr always sets it, so you don't
need the 'err &&' ... part.
> +    result = 0;
> +  else
> +    result = 1;
> +  free(err);
> +  return result;
> +}
> +
> +static char **
> +blkid_with_p_opt(const char *device)
>  {
>    int r;
>    char *out = NULL, *err = NULL;
>    char **lines = NULL;
> -
>    char **ret = NULL;
>    int size = 0, alloc = 0;
>  
> -  const char *blkid[] = {"blkid", "-p",
"-i", "-o", "export", device, NULL};
> -  r = commandv(&out, &err, blkid);
> +  r = command(&out, &err, "blkid", "-c",
"/dev/null",
> +               "-p", "-i", "-o",
"export", device, NULL);
>    if (r == -1) {
>      reply_with_error("%s", err);
>      goto error;
> @@ -142,12 +166,12 @@ do_blkid(const char *device)
>      }
>    }
>  
> +  if (add_string(&ret, &size, &alloc, NULL) == -1) goto error;
> +
>    free(out);
>    free(err);
>    free(lines);
>  
> -  if (add_string(&ret, &size, &alloc, NULL) == -1) return
NULL;
> -
>    return ret;
>  
>  error:
> @@ -158,3 +182,45 @@ error:
>  
>    return NULL;
>  }
> +
> +static char **
> +blkid_without_p_opt(const char *device)
> +{
> +  char **ret = NULL;
> +  int size = 0, alloc = 0;
> +
> +  if (add_string(&ret, &size, &alloc, "TYPE") == -1)
goto error;
> +  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"TYPE")) == -1)
> +    goto error;
> +  if (add_string(&ret, &size, &alloc, "LABEL") ==
-1) goto error;
> +  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"LABEL")) == -1)
> +    goto error;
> +  if (add_string(&ret, &size, &alloc, "UUID") == -1)
goto error;
> +  if (add_string(&ret, &size, &alloc, get_blkid_tag(device,
"UUID")) == -1)
> +    goto error;
> +  if (add_string_nodup(&ret, &size, &alloc, NULL) == -1) goto
error;
> +
> +  return ret;
> +error:
> +  if (ret) free_strings(ret);
> +  return NULL;
> +}
> +
> +char **
> +do_blkid(const char *device)
> +{
> +  int r;
> +  char *out = NULL, *err = NULL;
> +  char **lines = NULL;
> +
> +  char **ret = NULL;
> +  int size = 0, alloc = 0;
> +  int blkid_has_p_opt = -1;
^ blkid_has_p_opt shouldn't be initialized, since we are about to
unconditionally assign to it here:
> +  if ((blkid_has_p_opt = test_blkid_p_opt()) == -1)
> +    return NULL;
> +  else if (blkid_has_p_opt)
> +    return blkid_with_p_opt(device);
> +  else
> +    return blkid_without_p_opt(device);
> +}
> -- 
> 1.7.8.rc4
OK, but can you fix those things.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org