On Thu, Sep 27, 2012 at 09:19:34AM +0200, Paolo Bonzini wrote:
> >
> > 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.
Yes it was 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.
Haven't seen that, but yes can't depend on that I suppose.
>
> >>> 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, "/");
Ok, this is how its looking finally...
char *p, *q;
p = q = path + strspn(path, "/");
p += strcspn(p, "/");
if (*p == '\0') {
return -EINVAL;
}
gconf->volname = g_strndup(q, p - q);
p += strspn(p, "/");
if (*p == '\0') {
return -EINVAL;
}
gconf->image = g_strdup(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?
gluster://server/testvol//dir/a.img ?
Isn't this a valid URI ? Here volname=testvol and image=/dir/a.img.
If that's valid and needs to be supported, then the above strspn based
parsing logic needs some rewrite.
Regards,
Bharata.
