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 <string.h>
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
