Re: [libvirt] [PATCH 00/11] Revisit xen driver Coverity cleanup changes
On 01/30/2013 01:51 PM, John Ferlan wrote:
src/xen/xen_hypervisor.c | 220 ++++++++--------------------------
src/xen/xen_inotify.c | 48 ++++----
src/xen/xend_internal.c | 303 ++++++++++++++---------------------------------
src/xen/xm_internal.c | 195 ++++++++++++++----------------
src/xen/xs_internal.c | 239 +++++++++++--------------------------
5 files changed, 325 insertions(+), 680 deletions(-)
Thanks for the reviews Osier and Jim. Rather than respond to each, I'll
summarize the consistent points. If I should send a v2, let me know.
w/r/t PrivatePtr on one line:
I was trying to go with the 80 column rule. For 99% they could fit on
one line if I removed one extra space. For a couple, spanning 2 lines
kept the 80 columns in effect. In looking at other drivers - I don't
think any of them typecast the *PrivatePtr)<var>->privateData, so I
could remove that too. I left them in only because they were there.
w/r/t function headers:
I was trying to be consistent amongst all the files and then more in
general with other drivers. I kept to the concept that if a header fits
within 80 columns then don't break up the line. I changed the files
where a header wouldn't fit into 80 columns to be:
function(type arg,
type2 arg2,
type3 arg3,
type4 arg4)
{
}
w/r/t: domain->name:
I wasn't sure with this one, so I had left it in. I removed them.
There are also checks for "domain->id != -1" - any thoughts on those?
Those seem to be running or not checks, right? Seems as though the
virDomainObjIsActive() or hvirCheckFlags() is used elsewhere.
What about priv->handle in xen_hypervisor.c? Seems as though open will
set it and everyone else checks it. Given how these drivers call between
each other, I'm inclined to leave as it, but I'm not against removing
them either.
John