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@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