[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH for-4.13] xen/arm: Don't use _end in is_xen_fixed_mfn()
Hi, On 16/10/2019 11:18, Ian Jackson wrote: Stefano Stabellini writes ("Re: [PATCH for-4.13] xen/arm: Don't use _end in is_xen_fixed_mfn()"):My suggestion is going to work: "the compiler sees through casts" referred to comparisons between pointers, where we temporarily casted both pointers to integers and back to pointers via a MACRO. That case was iffy because the MACRO was clearly a workaround the spec. Here the situation is different. For one, we are doing arithmetic. Also virt_to_maddr already takes a vaddr_t as argument. So instead of doing pointers arithmetic, then converting to vaddr_t, we are converting to vaddr_t first, then doing arithmetics, which is fine both from a C99 point of view and even a MISRA C point of view. I can't see a problem with that. I am sure as I reasonable can be :-)FTAOD I think you are suggesting this: - + (mfn_to_maddr(mfn) <= virt_to_maddr(_end - 1))) + + (mfn_to_maddr(mfn) <= virt_to_maddr(((vaddr_t)_end - 1))) virt_to_maddr(va) is a macro which expands to __virt_to_maddr((vaddr_t)(va)) So what is happening here is that the cast to an integer type is being done before the subtraction. Without the cast, you are calculating the pointer value _end-1 from the value _end, which is UB. With the cast you are calculating an integer value. vaddr_t is unsigned, so all arithmetic operations are defined. Nothing casts the result back to the "forbidden" (with this provenance) pointer value, so all is well. (With the macro expansion the cast happens twice. This is probably better than using __virt_to_maddr here.) Ie, in this case I agree with Stefano. The cast is both necessary and sufficient. Thank you both for the explanation. I will update the patch and resend it. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |