From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56549) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TH8Nv-0005qa-C3 for qemu-devel@nongnu.org; Thu, 27 Sep 2012 03:19:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TH8Nt-0007SI-G7 for qemu-devel@nongnu.org; Thu, 27 Sep 2012 03:19:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55668) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TH8Nt-0007SE-8F for qemu-devel@nongnu.org; Thu, 27 Sep 2012 03:19:45 -0400 Message-ID: <5063FE06.3070304@redhat.com> Date: Thu, 27 Sep 2012 09:19:34 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <20120924091008.GJ18470@in.ibm.com> <20120924091340.GN18470@in.ibm.com> <5062D24F.7030304@redhat.com> <20120926161132.GA32195@in.ibm.com> <50632F6A.5020202@redhat.com> <20120927064132.GB18285@in.ibm.com> In-Reply-To: <20120927064132.GB18285@in.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 4/4] block: Support GlusterFS as a QEMU block backend. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: bharata@linux.vnet.ibm.com Cc: Kevin Wolf , Anthony Liguori , Anand Avati , Vijay Bellur , Stefan Hajnoczi , Harsh Bora , Amar Tumballi , qemu-devel@nongnu.org, "Richard W.M. Jones" , Blue Swirl , Avi Kivity , Daniel Veillard Il 27/09/2012 08:41, Bharata B Rao ha scritto: >>> As you note, I don't need 2nd strtok strictly since the rest of the string >>> is available in saveptr. But I thought using saveptr is not ideal or preferred. >>> I wanted to use the most appropriate/safe delimiter to extract the image string >>> in the 2nd strtok and decided to use '?'. >> >> I don't think it is defined what saveptr points to. >> http://publib.boulder.ibm.com/infocenter/pseries/v5r3/index.jsp?topic=/com.ibm.aix.basetechref/doc/basetrf2/strtok_r.htm >> says "the strtok_r subroutine also updates the Pointer parameter with >> the starting address of the token following the first occurrence of the >> Separators parameter". I read this as: >> >> *saveptr = token + strlen(token) + 1; >> >> which is consistent with this strtok example from the C standard: >> >> #include >> static char str[] = "?a???b,"; >> char *t; >> >> t = strtok(str, "?"); // t points to the token "a" >> t = strtok(str, ","); // t points to the token "??b" >> >> Have you tested this code with multiple consecutive slashes? > > Yes, both 2-strtok and strtok+saveptr approach work correctly (= as per > my expectation!) with multiple consecutive slashes. I do understand that > 2-strtok approach will not work when we have %3F in the path component of > the URI. > > For URIs like gluster://server/volname/path/to/image, both the approaches > extract image as "path/to/image". > > For URIs like gluster://server/volname//path/to/image, both the approaches > extract image as "/path/to/image". > > For gluster://server/volname////path/to/image, the image is extracted as > "//path/to/image". Should there be three /'s here? I assume it's just a typo. I'm concerned that there is no documentation of what saveptr actually points to. In many cases the strtok specification doesn't leave much free room, but in the case you're testing here: >>> /* image */ >>> if (!*saveptr) { >>> return -EINVAL; >>> } strtok_r may just as well leave saveptr = NULL for example. >>> gconf->image = g_strdup(saveptr); >>> >> >> I would avoid strtok_r completely: >> >> char *p = path + strcpsn(path, "/"); >> if (*p == '\0') { >> return -EINVAL; >> } >> gconf->volname = g_strndup(path, p - path); >> >> p += strspn(p, "/"); >> if (*p == '\0') { >> return -EINVAL; >> } >> gconf->image = g_strdup(p); > > This isn't working because for a URI like > gluster://server/volname/path/to/image, uri_parse() will give > "/volname/path/to/image" in uri->path. I would have expected to see > uri->path as "volname/path/to/image" (without 1st slash). Ok, that's easy enough to fix with an extra strspn, char *p = path + strpsn(path, "/"); p += strcspn(p, "/"); > Note that gluster is currently able to resolve image paths that don't > have a preceding slash (like dir/a.img). But I guess we should support > specifying complete image paths too (like /dir/a.img) How would the URIs look like? Paolo