[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v2] support setting bandwidth from virsh attach-interface(was Re: [PATCH] support setting bandwidth from virsh attach-interface)



> > +/* parse inbound and outbound which are in the format of
> > + * 'average,peak,burst', in which peak and burst are optional,
> > + * thus 'average,,burst' and 'average,peak' are also legal. */
> > +static int parseRateStr(const char *rateStr, virRatePtr rate)
> > +{
> > +    char *average = NULL, *peak = NULL, *burst = NULL;
> > +
> > +    average = (char *)rateStr;
> 
> I'd vote for const correctness here;
> So, either change average to be const char *, or leave it out and
> use passed rateStr directly instead.

used const char * in v3.

> > +    if (inboundStr || outboundStr) {
> > +        virBufferAsprintf(&buf, "  <bandwidth>\n");
> > +        if (inboundStr && inbound.average > 0) {
> > +            virBufferAsprintf(&buf, "    <inbound average='%lld'", inbound.average);
> > +            if (inbound.peak > 0)
> > +                virBufferAsprintf(&buf, " peak='%lld'", inbound.peak);
> > +            if (inbound.burst > 0)
> > +                virBufferAsprintf(&buf, " burst='%lld'", inbound.burst);
> > +            virBufferAsprintf(&buf, "/>\n");
> > +        }
> > +        if (outboundStr && outbound.average > 0) {
> > +            virBufferAsprintf(&buf, "    <outbound average='%lld'", outbound.average);
> > +            if (outbound.peak > 0)
> > +                virBufferAsprintf(&buf, " peak='%lld'", outbound.peak);
> > +            if (outbound.burst > 0)
> > +                virBufferAsprintf(&buf, " burst='%lld'", outbound.burst);
> > +            virBufferAsprintf(&buf, "/>\n");
> > +        }
> > +        virBufferAsprintf(&buf, "  </bandwidth>\n");
> > +    }
> > +
> 
> Since [in|out]bound average, peak and burst are defined as unsigned long
> long, you can actually check for (outbound.peak) instead of
> (outbound.peak > 0), but you can leave it as-is. But I'd prefer to
> change print format from '%lld' to '%llu'. I am surprised my gcc does
> not warn about it.

changed format from '%lld' to '%llu'.

> 
> ACK with those nits fixed.

Thanks for you comments.

-- 
Thanks,
Hu Tao


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]