From: Jon Maloy <maloy@donjonn•com>
To: unlisted-recipients:; (no To-header on input)
Cc: jon.maloy@ericsson•com, paul.gortmaker@windriver•com,
erik.hugne@ericsson•com, ying.xue@windriver•com,
tipc-discussion@lists•sourceforge.net, netdev@vger•kernel.org,
David Miller <davem@davemloft•net>
Subject: Re: [PATCH net-next 2/3] tipc: message reassembly using fragment chain
Date: Mon, 28 Oct 2013 06:24:09 -0400 [thread overview]
Message-ID: <526E3B49.9030506@donjonn.com> (raw)
In-Reply-To: <20131028.010737.400887499395331496.davem@davemloft.net>
On 10/28/2013 01:07 AM, David Miller wrote:
> From: Jon Maloy <jon.maloy@ericsson•com>
> Date: Sat, 26 Oct 2013 14:41:02 -0400
>
>> + int ret = tipc_link_recv_fragment(
>> + &node->bclink.reasm_head,
>> + &node->bclink.reasm_tail,
>> + &buf);
> This is not the correct way to indent a function call that spans
> multiple lines. In such a situation the arguments that appear
> on the second and subsequent lines must start at the first column
> after the openning parenthesis of the function call.
>
> Like this:
>
> func(a, b, c,
> d, e, f);
>
> Please audit this in your entire set of patches and resubmit,
> thanks.
Doing as David says here means that some lines will be >80 chars.
This was the reason for the somewhat strange indentation.
I tried to rename the function to tipc_link_rcv_fragm(), but one
line was still too long. The problem we have goes deeper.
In Linus' coding style manual I read that the 80 char limit is a hard limit,
a limit we violate in several places. One offender is that we have too
many indentation levels, at least in tipc_recv_msg() and probably in
some other places. This is sensitive code, that I don't feel for touching
right now. A more low hanging fruit is our local variable names:
names such as l_ptr, n_ptr, b_ptr is exactly what Linus characterizes
as "brain dead Hungarian style", and I never liked that naming anyway.
For me l, n, and b is good enough as long as the context is clear.
But, doing so, at least in tipc_recv_msg(), would require another, separate,
patch, and it would lead to style inconsistency.
In brief, I am at loss about to proceed here, and I am not going to submit
this patch again until I have some feedback from somebody who can tell me
what is the right thing to do. Maybe > 80 chars is fine for now?
///jon
next prev parent reply other threads:[~2013-10-28 10:24 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-26 18:41 [PATCH net-next 0/3] message reassembly improvements Jon Maloy
2013-10-26 18:41 ` [PATCH net-next 1/3] tipc: don't reroute message fragments Jon Maloy
2013-10-26 18:41 ` [PATCH net-next 2/3] tipc: message reassembly using fragment chain Jon Maloy
2013-10-28 5:07 ` David Miller
2013-10-28 10:24 ` Jon Maloy [this message]
2013-10-29 1:56 ` Ying Xue
2013-10-29 3:49 ` David Miller
2013-10-29 5:36 ` Ying Xue
2013-10-26 18:41 ` [PATCH net-next 3/3] tipc: reassembly failures should cause link reset Jon Maloy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=526E3B49.9030506@donjonn.com \
--to=maloy@donjonn$(echo .)com \
--cc=davem@davemloft$(echo .)net \
--cc=erik.hugne@ericsson$(echo .)com \
--cc=jon.maloy@ericsson$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=paul.gortmaker@windriver$(echo .)com \
--cc=tipc-discussion@lists$(echo .)sourceforge.net \
--cc=ying.xue@windriver$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox