Messages in this thread | | | Date | Wed, 9 Jan 2019 07:37:48 +0900 | From | Stafford Horne <> | Subject | Re: [PATCH] arch/openrisc: Fix issues with access_ok() |
| |
On Tue, Jan 08, 2019 at 10:16:39AM -0800, Linus Torvalds wrote: > On Tue, Jan 8, 2019 at 10:10 AM Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > On Tue, Jan 8, 2019 at 5:15 AM Stafford Horne <shorne@gmail.com> wrote: > > > > > > The commit 594cc251fdd0 ("make 'user_access_begin()' do 'access_ok()'") > > > exposed incorrect implementations of access_ok() macro in several > > > architectures. This change fixes 2 issues found in OpenRISC. > > > > Looks good to me. Should I apply this directly, or expect a pull > > request with it later? > > Oh, and replying to myself with a quick note: it might also be a good > idea to just make it an inline function. > > The only reason I did alpha and SH as those macros with a statement > expression was that because I don't have a cross-build environment, > continuing to do it as a macro was the safest thing from a build > standpoint. > > One big difference between a macro and an inline function is that the > inline function requires everything to be declared at the point of the > function definition, while the macro can use things that get declared > only later (like "get_fs()"). So a macro can use functions and other > macros that aren't declared yet, but will be declared by the time the > macro is actually _used_. > > So when changing the macro "blind", it was simply safer to just keep > it a macro and just make it a bit more complex. But since you can > build-test your changes, making (for example) __range_ok() be an > inline function might have been the cleaner solution to the "use > twice" issue. > > But your existing patch looks fine to me too, so don't worry too much > about it. I just wanted to point out that the reason I did alpha and > SH the way I did was not some "macros are better", but rather "Linus > is weak and lazy".
Hi Linus,
Please take this patch directly.
The inline's are a good point. I will take some time to address this and some other cleanups.
Note (for others) I had to apply patches from Masahiro Yamada to test this as v5.0-rc1 build was broken for OpenRISC. Those patches are already on Linus' master.
-Stafford
| |