| Message ID | 1457019043-14050-1-git-send-email-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Wei Liu writes ("[PATCH] libxc: initialise rc to -1 at the beginning of meminit_hvm"):
> Variable rc is only set either inside a loop or inside some if
> statements. To avoid confuse gcc with stricter setting we set rc to -1
> at the beginning.
Is this really the best way to fix this ?
AFAICT the problem arises only for this code:
if ( rc != 0 )
{
DOMPRINTF("Could not allocate memory for HVM guest.");
goto error_out;
}
which occurs after the loop. But that path happens only if
if ( rc != 0 )
break;
is executed inside the loop.
If the error case was moved into the loop, there would be no need for
the separate test of rc (which is confusing to humans as well as to
the compiler).
Do you agree ?
Ian.
On Thu, Mar 03, 2016 at 03:46:23PM +0000, Ian Jackson wrote: > Wei Liu writes ("[PATCH] libxc: initialise rc to -1 at the beginning of meminit_hvm"): > > Variable rc is only set either inside a loop or inside some if > > statements. To avoid confuse gcc with stricter setting we set rc to -1 > > at the beginning. > > Is this really the best way to fix this ? > > AFAICT the problem arises only for this code: > > if ( rc != 0 ) > { > DOMPRINTF("Could not allocate memory for HVM guest."); > goto error_out; > } > > which occurs after the loop. But that path happens only if > > if ( rc != 0 ) > break; > > is executed inside the loop. > > If the error case was moved into the loop, there would be no need for > the separate test of rc (which is confusing to humans as well as to > the compiler). > > Do you agree ? > Yes, that's better. I will send v2 shortly. Wei. > Ian.
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c index e13a4aa..c0c01a0 100644 --- a/tools/libxc/xc_dom_x86.c +++ b/tools/libxc/xc_dom_x86.c @@ -1258,7 +1258,7 @@ static int meminit_hvm(struct xc_dom_image *dom) unsigned long p2m_size; unsigned long target_pages = dom->target_pages; unsigned long cur_pages, cur_pfn; - int rc; + int rc = -1; xen_capabilities_info_t caps; unsigned long stat_normal_pages = 0, stat_2mb_pages = 0, stat_1gb_pages = 0;
Variable rc is only set either inside a loop or inside some if statements. To avoid confuse gcc with stricter setting we set rc to -1 at the beginning. Reported-by: Doug Goldstein <cardoe@cardoe.com> Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Cc: Roger Pau Monne <roger.pau@citrix.com> --- tools/libxc/xc_dom_x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)