On 22/06/2017 09:14, Jan Beulich wrote: >>>> On 21.06.17 at 17:12, <andrew.coop...@citrix.com> wrote: >> sh_emulate_map_dest() predates the introduction of the generic ERR_PTR() >> infrasturcture, but take the opportunity to avoid opencoding it. >> >> The chosen error constants require need to be negative to work with IS_ERR(), >> but no other changes. > Drop one of "require" or "need"? > >> --- a/xen/arch/x86/mm/shadow/private.h >> +++ b/xen/arch/x86/mm/shadow/private.h >> @@ -395,10 +395,9 @@ void shadow_unhook_mappings(struct domain *d, mfn_t >> smfn, int user_only); >> >> /* Returns a mapped pointer to write to, or one of the following error >> * indicators. */ >> -#define MAPPING_UNHANDLEABLE ((void *)(unsigned long)X86EMUL_UNHANDLEABLE) >> -#define MAPPING_EXCEPTION ((void *)(unsigned long)X86EMUL_EXCEPTION) >> -#define MAPPING_SILENT_FAIL ((void *)(unsigned long)X86EMUL_OKAY) >> -#define sh_emulate_map_dest_failed(rc) ((unsigned long)(rc) <= 3) >> +#define MAPPING_UNHANDLEABLE ERR_PTR(~(long)X86EMUL_UNHANDLEABLE) >> +#define MAPPING_EXCEPTION ERR_PTR(~(long)X86EMUL_EXCEPTION) >> +#define MAPPING_SILENT_FAIL ERR_PTR(~(long)X86EMUL_OKAY) > While this doesn't change with your patch, this last definition has a > string dependency on X86EMUL_OKAY to be zero, yet there's no > respective BUILD_BUG_ON() anywhere. Would you mind adding > one at once? In any event > Reviewed-by: Jan Beulich <jbeul...@suse.com>
There is no longer any dependence on X86EMUL_OKAY being 0 (so far as I can see). All this logic would work fine if OKAY had the value 10 for example. The only requirement (now) is that the X86EMUL constants are all within MAX_ERRNO so they count as errors as far as the ERR_PTR logic is concerned, and I suppose those should gain BUILD_BUG_ON()s ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
