Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH 1/2] Define find_symbol_in_section_t as function type to simplify the code | Date | Thu, 16 Jul 2015 20:59:47 +0930 |
| |
Andrew Morton <akpm@linux-foundation.org> writes: > On Wed, 15 Jul 2015 07:22:32 +0930 Rusty Russell <rusty@rustcorp.com.au> wrote: >> It's shorter, but it's less clear. typedefs on functions are not very >> useful: >> 1) They require readers to look in two places to see how to use the >> function (ie each_symbol_section). >> 2) They can't use the typedef to declare their function, since that >> doesn't work in C. >> >> If the function were being used many times, it makes sense. But >> it's only used twice, once static inside module.c. >> > > Using a foo_t typedef for a function callback is a common pattern. > It's (almost) the only approved use of typedefs. The usage is > widespread enough that when one sees a foo_t type, one says "ahah, > that's a function pointer".
I always thought of a type which can map to varying types under different arch/configs as the typical typedef.
> Sorry, but I don't think "Rusty doesn't like it" is a good reason for > the module code to be different.
But "Rusty has to maintain it" is a pretty strong counter argument, IMHO.
> All of us dislike some aspects of > kernel coding practices, but we go along because consistency is more > important.
Consistency is important when it makes things more readable, sure.
I don't think any kernel devs are going to get confused seeing a function pointer, and I think this patch makes the code slightly less readable.
Enough not to apply the patch, but not enough waste more time on it.
Cheers, Rusty.
| |