On Mon, 2017-12-11 at 14:43 -0800, Matthew Wilcox wrote:
> On Mon, Dec 11, 2017 at 02:12:28PM -0800, Joe Perches wrote:
> > Completely reasonable. Thanks.
>
> If we're doing "completely reasonable" complaints, then ...
>
> - I don't understand why plain 'unsigned' is deemed bad.
That was a David Miller preference.
> - The rule about all function parameters in prototypes having a name
> doesn't make sense. Example:
>
> int ida_get_new_above(struct ida *ida, int starting_id, int *p_id);
Improvements to regex welcomed.
> - Forcing a blank line after variable declarations sometimes makes for
> some weird-looking code.
True. I don't care for this one myself.
> Constructively, I think this warning can be suppressed for blocks
> that are under, say, 8 lines.
Not easy to do as checkpatch works on patches.
> 6) Functions
> ------------
>
> Functions should be short and sweet, and do just one thing. They should
> fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
> as we all know), and do one thing and do that well.
>
> I'm not expecting you to be able to write a perl script that checks
> the first line, but we have way too many 200-plus line functions in
> the kernel. I'd like a warning on anything over 200 lines (a factor
> of 4 over Linus's stated goal).
Maybe reasonable.
Some declaration blocks for things like:
void foo(void)
{
static const struct foobar array[] = {
{ long count of lines... };
[body]
}
might make that warning unreasonable though.
> - I don't understand the error for xa_head here:
>
> struct xarray {
> spinlock_t xa_lock;
> gfp_t xa_flags;
> void __rcu * xa_head;
> };
>
> Do people really think that:
>
> struct xarray {
> spinlock_t xa_lock;
> gfp_t xa_flags;
> void __rcu *xa_head;
> };
>
> is more aesthetically pleasing? And not just that, but it's an *error*
> so the former is *RIGHT* and this is *WRONG*. And not just a matter
> of taste?
No opinion really.
That's from Andy Whitcroft's original implementation.
