On Thu, Feb 18, 2016 at 7:18 AM, Andrew Cooper <andrew.coop...@citrix.com> wrote:
> On 18/02/16 14:16, Razvan Cojocaru wrote: > > On 02/18/2016 04:10 PM, Tamas K Lengyel wrote: > >> On Feb 18, 2016 03:46, "Razvan Cojocaru" <rcojoc...@bitdefender.com > >> <mailto:rcojoc...@bitdefender.com>> wrote: > >>> On 02/18/2016 12:45 PM, Corneliu ZUZU wrote: > >>>> c/s f5365e6: "xen/vm-events: Move parts of monitor_domctl code to > >> common-side", > >>>> introduced a use without initialization issue. > >>>> hvm_event_breakpoint calls hvm_event_traps(&req) and if sync is true > >> that > >>>> ors some bits into req->flags which was never initialised. > >>>> Reported by Coverity Scan. > >>>> > >>>> Initializes req @ hvm_event_breakpoint entry. > >>>> > >>>> Signed-off-by: Corneliu ZUZU <cz...@bitdefender.com > >> <mailto:cz...@bitdefender.com>> > >>>> --- > >>>> xen/arch/x86/hvm/event.c | 2 +- > >>>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>>> > >>>> diff --git a/xen/arch/x86/hvm/event.c b/xen/arch/x86/hvm/event.c > >>>> index 874a36c..cb9c152 100644 > >>>> --- a/xen/arch/x86/hvm/event.c > >>>> +++ b/xen/arch/x86/hvm/event.c > >>>> @@ -173,7 +173,7 @@ int hvm_event_breakpoint(unsigned long rip, > >>>> { > >>>> struct vcpu *curr = current; > >>>> struct arch_domain *ad = &curr->domain->arch; > >>>> - vm_event_request_t req; > >>>> + vm_event_request_t req = {}; > >> Should this be = { 0 } instead? Also, as I recall the request is not > >> initialized on any of the paths, so we might as well do it for all of > >> them, not just here. It would help avoid the listener erronously using > >> some fields that were not actually initialized as well. > > That should achieve the same thing. I first looked up "= {}" in the Xen > > source code and found: > > > > $ grep -R "= {}" > > arch/arm/domain_build.c: struct kernel_info kinfo = {}; > > arch/x86/time.c: struct vcpu_time_info *u, _u = {}; > > arch/x86/tboot.c: uint8_t nonce[16] = {}; > > arch/x86/tboot.c: uint8_t nonce[16] = {}; > > arch/x86/tboot.c: uint8_t nonce[16] = {}; > > arch/x86/traps.c: unsigned char insns_before[8] = {}, insns_after[16] = > {}; > > arch/x86/x86_emulate/x86_emulate.c: union vex vex = {}; > > arch/x86/x86_emulate/x86_emulate.c: struct x86_emulate_stub stub = {}; > > arch/x86/cpu/mtrr/generic.c:struct mtrr_state mtrr_state = {}; > > arch/x86/cpu/common.c:const struct cpu_dev *__read_mostly > > cpu_devs[X86_VENDOR_NUM] = {}; > > arch/x86/domain.c: unsigned long total[MASK_EXTR(PGT_type_mask, > > PGT_type_mask) + 1] = {}; > > tools/kconfig/expr.c: union string_value lval = {}, rval = {}; > > common/grant_table.c: struct gnttab_copy_buf src = {}; > > common/grant_table.c: struct gnttab_copy_buf dest = {}; > > > > So I thought that that's the prevailing convention. I've now searched > > for "= {0}", and I see that that has also been done, so I suppose either > > way is fine as far as the coding style goes? > > > > $ grep -R "= {0}" > > arch/arm/mm.c: lpae_t pte = {0}; > > arch/arm/mm.c: lpae_t pte = {0}; > > drivers/char/exynos4210-uart.c:} exynos4210_com = {0}; > > drivers/char/pl011.c:} pl011_com = {0}; > > drivers/char/omap-uart.c:} omap_com = {0}; > > drivers/char/scif-uart.c:} scif_com = {0}; > > drivers/char/cadence-uart.c:} cuart_com = {0}; > > tools/kconfig/nconf.gui.c:attributes_t attributes[ATTR_MAX+1] = {0}; > > crypto/vmac.c: uint64_t in[2] = {0}, out[2]; > > The two are equivalent as far as C goes. I prefer the former as it is > slightly shorter. OK, as long as they are equivalent I'm fine with either. I always use the {0} syntax, it looks more explicit to me, so just wanted to double-check that it means the same thing. Thanks, Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
