Messages in this thread |  | | Date | Fri, 3 Aug 2018 16:39:36 +0200 | From | Andrew Lunn <> | Subject | Re: [PATCH v1 3/3] net: WireGuard secure network tunnel |
| |
On Fri, Aug 03, 2018 at 02:35:40AM +0200, Jason A. Donenfeld wrote: > On Tue, Jul 31, 2018 at 10:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > > I just gave this patch to checkpatch.pl... > On Tue, Jul 31, 2018 at 10:22 PM Stephen Hemminger > <stephen@networkplumber.org> wrote: > > Please break lines at something reasonable like 100 characters. > > If the long lines really truly are dreadful, I have no problem fixing > that up for v2.
A very large percentage of the kernel code uses 80 characters maximum. So anything which does not makes it look different. It makes it not look like kernel code. It is also not just about styling. If you use long lines, it makes it easier to use more indentation levels. But good readable code tends to have lots of little functions with only one or two levels of indentation, and each function does just one easily understood thing.
Plus this is pretty much an open invite for kernel newbies to submit patches fixing up all these warnings. Maybe you would prefer to do it yourself, so you keep some degree of control over it. Also, such white space changes make backporting fixes more difficult. So it is better if you can get this done before it lands in the kernel and backporting becomes necessary.
> On Tue, Jul 31, 2018 at 10:02 PM Andrew Lunn <andrew@lunn.ch> wrote: > > > +static __always_inline void swap_endian(u8 *dst, const u8 *src, u8 bits) > > There is a general preference to not force the compile to > > inline. Leave it to decide. > > I'm aware this is for the most part the case, and I've read the > variety of threads and documentation of folks explaining why this is a > good policy. In the particular instance of that function, inlining is > in fact always the right thing to do. But I'll give it a double check > to see if the compiler is already figuring that out on its own.
Well, what compiler are you going to check? I think the minimum version of GCC is now v4.5. So you probably want to check 8.2, 7.3, 6.4, 5.5, 4.9.3 and 4.8. And you might want to check ARM, MIPs, x86 and amd64. So only 24 versions.
More realistically, if you have a strong argument why a particular function must be inlined, e.g. to avoid a timing attack, make that argument.
Andrew
|  |