[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 7/7] xen/memory: Fix mapping grant tables with XENMEM_acquire_resource
On 29.01.2021 19:18, Andrew Cooper wrote: > On 29/01/2021 09:46, Jan Beulich wrote: >> On 29.01.2021 00:44, Andrew Cooper wrote: >>> On 15/01/2021 16:12, Jan Beulich wrote: >>>> On 12.01.2021 20:48, Andrew Cooper wrote: >>>>> --- a/xen/arch/x86/mm.c >>>>> +++ b/xen/arch/x86/mm.c >>>>> @@ -4628,7 +4628,6 @@ int arch_acquire_resource(struct domain *d, >>>>> unsigned int type, >>>>> if ( id != (unsigned int)ioservid ) >>>>> break; >>>>> >>>>> - rc = 0; >>>>> for ( i = 0; i < nr_frames; i++ ) >>>>> { >>>>> mfn_t mfn; >>>> How "good" are our chances that older gcc won't recognize that >>>> without this initialization ... >>>> >>>>> @@ -4639,6 +4638,9 @@ int arch_acquire_resource(struct domain *d, >>>>> unsigned int type, >>>>> >>>>> mfn_list[i] = mfn_x(mfn); >>>>> } >>>>> + if ( i == nr_frames ) >>>>> + /* Success. Passed nr_frames back to the caller. */ >>>>> + rc = nr_frames; >>>> ... rc will nevertheless not remain uninitialized when nr_frames >>>> is zero? >>> I don't anticipate this function getting complicated enough for us to >>> need to rely on tricks like that to spot bugs. >>> >>> AFAICT, it would take a rather larger diffstat to make it "uninitialised >>> clean" again. >> Well, we'll see how it goes then. We still allow rather ancient >> gcc, and I will eventually run into a build issue here once >> having rebased accordingly, if such old gcc won't cope. > > But those problematic compilers have problems spotting that a variable > is actually initialised on all paths. > > This case is opposite. It is unconditionally initialised to -EINVAL > (just outside of the context above), which breaks some "the compiler > would warn us about error paths" logic that we try to use. Oh, right you are. I'm sorry for the noise then. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |