mbox series

[net-next,00/29] l2tp: cleanup checkpatch.pl warnings

Message ID 20200721173221.4681-1-tparkin@katalix.com
Headers show
Series l2tp: cleanup checkpatch.pl warnings | expand

Message

Tom Parkin July 21, 2020, 5:31 p.m. UTC
l2tp hasn't been kept up to date with the static analysis checks offered
by checkpatch.pl.

This patchset makes l2tp checkpatch-clean, with some exceptions as
described below.

The majority of these issues are trivial: for example small whitespace
tweaks, comment style, line breaks and indentation.  The first patches
in the series deal with these.

The more substantive patches fall into two categories:

 * Some rearrangement of code to better deal with CONFIG_IPV6 ifdefs,
   thereby avoiding checkpatch warnings.

 * Removal of BUG_ON in l2tp.  Some uses of BUG_ON offered little or no
   benefit in the current codebase: these have been removed.  Others do
   help to flag an unexpected condition; and these have been replaced
   with a WARN_ON and appropriate recovery code.

The following checkpatch warnings are not addressed:

 * Two line length warnings for lines of 103 and 101 characters.  These
   lines are more readable without a linebreak so I prefer to leave them
   as-is.

 * Two warnings about unbalanced braces around an else statement in
   l2tp_core.c.  These result from conditionally-compiled code providing
   IPv6 support.  In other areas I have resolved similar warnings by
   breaking code out into functions, but in these remaining cases it'd
   be more difficult to do this without significant modifications to the
   code.  For this patchset at least I prefer to not make such invasive
   changes.

 * Two warnings about use of ENOSYS in l2tp_ppp.c.  This error code is
   returned when session-specific ioctl calls are made on a tunnel socket.
   Arguably a different code would be more appropriate; however changing
   this could be seen to be changing the userspace API.  It's unlikely
   that any code is relying on this particular error being returned in
   this scenario, but again, I prefer not to make such a change in this
   patchset.

Tom Parkin (29):
  l2tp: cleanup whitespace around operators
  l2tp: tweak comment style for consistency
  l2tp: add a blank line following declarations
  l2tp: cleanup excessive blank lines
  l2tp: cleanup difficult-to-read line breaks
  l2tp: cleanup wonky alignment of line-broken function calls
  l2tp: cleanup suspect code indent
  l2tp: add identifier name in function pointer prototype
  l2tp: prefer using BIT macro
  l2tp: cleanup comparisons to NULL
  l2tp: cleanup unnecessary braces in if statements
  l2tp: prefer seq_puts for unformatted output
  l2tp: avoid multiple assignments
  l2tp: line-break long function prototypes
  l2tp: comment per net spinlock instances
  l2tp: fix up incorrect comment in l2tp_recv_common
  l2tp: avoid precidence issues in L2TP_SKB_CB macro
  l2tp: WARN_ON rather than BUG_ON in l2tp_debugfs.c
  l2tp: use a function to render tunnel address in l2tp_debugfs
  l2tp: cleanup netlink send of tunnel address information
  l2tp: cleanup netlink tunnel create address handling
  l2tp: cleanup kzalloc calls
  l2tp: remove BUG_ON in l2tp_session_queue_purge
  l2tp: remove BUG_ON in l2tp_tunnel_closeall
  l2tp: don't BUG_ON session magic checks in l2tp_ppp
  l2tp: don't BUG_ON seqfile checks in l2tp_ppp
  l2tp: WARN_ON rather than BUG_ON in l2tp_session_queue_purge
  l2tp: remove BUG_ON refcount value in l2tp_session_free
  l2tp: WARN_ON rather than BUG_ON in l2tp_session_free

 net/l2tp/l2tp_core.c    | 123 +++++++++----------
 net/l2tp/l2tp_core.h    |  82 ++++++-------
 net/l2tp/l2tp_debugfs.c |  66 ++++++-----
 net/l2tp/l2tp_eth.c     |  19 ++-
 net/l2tp/l2tp_ip.c      |  31 +++--
 net/l2tp/l2tp_ip6.c     |  37 +++---
 net/l2tp/l2tp_netlink.c | 257 +++++++++++++++++++++-------------------
 net/l2tp/l2tp_ppp.c     |  97 ++++++++-------
 8 files changed, 362 insertions(+), 350 deletions(-)

Comments

David Miller July 21, 2020, 11:19 p.m. UTC | #1
This patch set is way too large to be reasonably reviewed by other
developers.

Please either find a way to combine some of the patches, or submit
this in stages of about 10 or so patches at a time.

I am not applying this submission as submitted.

Thank you.
Tom Parkin July 22, 2020, 9:07 a.m. UTC | #2
On  Tue, Jul 21, 2020 at 16:19:17 -0700, David Miller wrote:
> 
> This patch set is way too large to be reasonably reviewed by other
> developers.
> 
> Please either find a way to combine some of the patches, or submit
> this in stages of about 10 or so patches at a time.
> 
> I am not applying this submission as submitted.
> 
> Thank you.

I will respin as requested, probably covering the more trivial
modifications in one series and the larger changes in a second
separate series.

Thanks for looking at it.