Re: [PATCH v9 for Xen 4.7 3/4] libxl: enable per-VCPU parameter for RTDS

From: Dario Faggioli <dario.faggioli@citrix.com>
To: Wei Liu <wei.liu2@citrix.com>, Chong Li <lichong659@gmail.com>
Cc: Chong Li <chong.li@wustl.edu>, Sisu Xi <xisisu@gmail.com>,
	George Dunlap <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	xen-devel <xen-devel@lists.xen.org>,
	Meng Xu <mengxu@cis.upenn.edu>,
	Dagaen Golomb <dgolomb@seas.upenn.edu>
Subject: Re: [PATCH v9 for Xen 4.7 3/4] libxl: enable per-VCPU parameter for RTDS
Date: Wed, 6 Apr 2016 22:01:59 +0200	[thread overview]
Message-ID: <1459972919.3166.114.camel@citrix.com> (raw)
In-Reply-To: <20160406194945.GB14830@citrix.com>

[-- Attachment #1.1: Type: text/plain, Size: 2327 bytes --]

On Wed, 2016-04-06 at 20:49 +0100, Wei Liu wrote:
> On Wed, Apr 06, 2016 at 02:41:31PM -0500, Chong Li wrote:
> > On Wed, Apr 6, 2016 at 2:30 PM, Wei Liu <wei.liu2@citrix.com>
> > wrote:
> > > On Wed, Apr 06, 2016 at 02:20:55PM -0500, Chong Li wrote:
> > > Not sure what kind of sanity check you were thinking about. But
> > > you can
> > > just set rc = 0 at the beginning of each function. That semantics
> > > should
> > > be sensible enough. What do you think?
> > Yes, I can do this change. But didn't you or Dario say that rc
> > should not be
> > initialized at the beginning of a function?
> > 
> I'm fine with bending the rules a bit to make life easier. For a
> simple
> function like this I won't argue one way or another. Besides there
> isn't
> really a CODING_STYLE file in libxc.
> 
I did say it, but it wasn't because of coding style, especially
considering, as Wei says, that we're in libxc. I did because it looked
a pointless (and hence avoidable) initialization.

Of course, I was wrong, as I did not see the issue in case we get
passed 0. Also, weirdly enough, my compiler did not spot this (and I
build on Debian unstable, so with a fairly recent gcc).

In any case, I don't think it make much sense to deal with num_vcpus=0,
and in fact, in libxl, we reject it. But as a matter of fact, the
easiest and cleanest way of "not dealing" with it, here in libxc, is
really to just initialize rc to zero, let the loop be skipped and carry
on.

More advanced sanity checking is better done at both higher and lower
levels, and both are happening already, so we're fine.

> But really what I care about is to not return an uninitialised rc. If
> you don't want to set rc to 0 at the beginning of the function, you
> can
> do it before the loop -- that should both fix the error and make
> Dario
> happy. :-)
> 
Thanks for this effort of making me happy. :-)

Jokes apart, just initialize it and get done with this. Again, it's the
cleanest and simplest way, and I _am_ happy with it.

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel