On 28.11.2023 14:17, Andrew Cooper wrote: > On 28/11/2023 1:00 pm, Jan Beulich wrote: >> On 28.11.2023 10:46, Federico Serafini wrote: >>> Uniform declaration and definition of guest_walk_tables() using >>> parameter name "pfec_walk": >>> this name highlights the connection with PFEC_* constants and it is >>> consistent with the use of the parameter within function body. >>> No functional change. >>> >>> Signed-off-by: Federico Serafini <federico.seraf...@bugseng.com> >> I'm curious what other x86 maintainers think. I for one don't like this, >> but not enough to object if others are happy. That said, there was earlier >> discussion (and perhaps even a patch), yet without a reference I don't >> think I can locate this among all the Misra bits and pieces. > > I looked at this and wanted a bit of time to think. > > Sadly, this code is half way through some cleanup, which started before > speculation and will continue in my copious free time. > > It's wrong to be passing PFEC_* constants, and that's why I renamed pfec > -> walk the last time I was fixing security bugs here (indeed, passing > the wrong constant here *was* the security issue). I missed the > prototype while fixing the implementation. > > At some point, PFEC_* will no longer be passed in. > > Therefore I'd far rather this was a one-line change for the declaration > changing pfec -> walk.
So that was what Federico originally had. Did you see my response at https://lists.xenproject.org/archives/html/xen-devel/2023-07/msg00122.html ? While I certainly agree with your plans (as far as I understand them), doing as you suggest would make it harder to spot what values are correct to pass to the function today. With a suitable new set of constants and perhaps even a proper typedef, such confusion would likely not arise. Jan
