Messages in this thread | | | From | Frank Lee <> | Date | Wed, 28 Nov 2018 08:57:30 +0800 | Subject | Re: [PATCH] x86/mm/dump_pagetables: Change to use DEFINE_SHOW_ATTRIBUTE macro |
| |
On Wed, Nov 28, 2018 at 6:50 AM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Nov 19, 2018 at 9:06 AM, Dave Hansen <dave.hansen@intel.com> wrote: > > On 11/19/18 7:43 AM, Yangtao Li wrote: > >> -static const struct file_operations ptdump_curusr_fops = { > >> - .owner = THIS_MODULE, > >> - .open = ptdump_open_curusr, > >> - .read = seq_read, > >> - .llseek = seq_lseek, > >> - .release = single_release, > >> -}; > >> +DEFINE_SHOW_ATTRIBUTE(ptdump_curusr); > > > > FWIW, I rather dislike this conversion and the DEFINE_SHOW_ATTRIBUTE() > > approach in general. It makes it basically impossible to go from > > ptdump_curusr to ptdump_open_curusr without opening up the macro and > > reverse-engineering it. > > > > My test is that for these macros to be sane, I need to be able to find > > "ptdump_open_curusr" by grepping for "ptdump_curusr". This fails the test. > > Er, "ptdump_curusr" matches the generated name "ptdump_curusr_show", > is that what you mean? > > > I don't think saving a few lines of code is worth the obfuscation. > > This is the standard boilerplate for attributes, though. I'd be nice > to drop all the copy/pasted code... > > -- > Kees Cook I also think that we should delete the duplicate code which doesn't make much sense.
MBR, Yangtao
| |