From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51886) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGucx-0005va-9T for qemu-devel@nongnu.org; Wed, 26 Sep 2012 12:38:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TGucv-0005va-JH for qemu-devel@nongnu.org; Wed, 26 Sep 2012 12:38:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11276) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TGucv-0005vK-9m for qemu-devel@nongnu.org; Wed, 26 Sep 2012 12:38:21 -0400 Message-ID: <50632F6A.5020202@redhat.com> Date: Wed, 26 Sep 2012 18:38:02 +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> In-Reply-To: <20120926161132.GA32195@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 26/09/2012 18:11, Bharata B Rao ha scritto: >>> +static int parse_volume_options(GlusterConf *gconf, char *path) >>> > > +{ >>> > > + char *token, *saveptr; >>> > > + >>> > > + /* volname */ >>> > > + token = strtok_r(path, "/", &saveptr); >>> > > + if (!token) { >>> > > + return -EINVAL; >>> > > + } >>> > > + gconf->volname = g_strdup(token); >>> > > + >>> > > + /* image */ >>> > > + token = strtok_r(NULL, "?", &saveptr); >> > >> > If I understand uri.c right, there is no ? in the path, so there's no >> > reason to call strtok. You could just use the rest of the string. Actually there could be a %3F which uri.c would unescape to ? (only the query part is left escaped), so your usage of strtok_r is incorrect. > 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? > If you think using saveptr is fine, then I could use that as below... > > /* image */ > if (!*saveptr) { > return -EINVAL; > } > 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); Paolo