Re: [Gluster-devel] [PATCH v2 1/2] Qemu/Gluster: Add Gluster protocol as supported network disk formats.

[Deepak C Shetty]

> > > +                /* if IPv6 addr, use square brackets to enclose it */
> > > +                virBufferAsprintf(opt, "[%s]:%s", disk->hosts->name,
> > > disk->hosts->port);
> > > +            } else {
> > > +                virBufferAsprintf(opt, "%s:%s", disk->hosts->name,
> > > disk->hosts->port);
> > > +            }
> > > +        }
> > > +
> > > +        /* append source path to gluster disk image */
> > > +        virBufferAsprintf(opt, "/%s", disk->src);
> > > +
> > > +        /* if transport type is unix, server name is path to unix
> > > socket, ignore port */
> > > +        if (disk->hosts->transport ==
> > > VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) {
> > > +            virBufferAsprintf(opt, "?socket=%s", disk->hosts->name);
> > > +        }
> > This can be clubbed as part of else clause to the above if condn (if
> > (disk->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX)), that ways
> > we avoid an un-necessary check of transport here.
> > It also means that disk->src needs to be pulled inside the if & else
> > clauses, which I feel should be ok.
>
> That was the only reason I kept it out of else. Isn't it better to 
> avoid redundant code than replacing if with an else ?
I think it can be coded to look non-redundant too.. you just have to use 
the disk->src inside the virBufferAsprintf, like you are using 
disk->hosts->name. I saw Dan's comment abt using virURIPtr, so maybe 
things change with that.