Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753542AbZLQXdX (ORCPT ); Thu, 17 Dec 2009 18:33:23 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751264AbZLQXdW (ORCPT ); Thu, 17 Dec 2009 18:33:22 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6504 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751247AbZLQXdU (ORCPT ); Thu, 17 Dec 2009 18:33:20 -0500 Date: Thu, 17 Dec 2009 18:33:12 -0500 (EST) From: Mikulas Patocka X-X-Sender: mpatocka@hs20-bc2-1.build.redhat.com To: Paul Mundt cc: linux-kernel@vger.kernel.org, Linus Torvalds , Alasdair G Kergon , dm-devel@redhat.com Subject: Re: [PATCH] Drop 80-character limit in checkpatch.pl In-Reply-To: <20091217231240.GA1421@linux-sh.org> Message-ID: References: <20091217061229.GD3946@linux-sh.org> <20091217231240.GA1421@linux-sh.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2909 Lines: 67 On Fri, 18 Dec 2009, Paul Mundt wrote: > On Thu, Dec 17, 2009 at 05:37:57PM -0500, Mikulas Patocka wrote: > > On Thu, 17 Dec 2009, Paul Mundt wrote: > > > On Tue, Dec 15, 2009 at 04:57:49PM -0500, Mikulas Patocka wrote: > > > > (5) Wrapping makes long expressions harder to understand > > > > -------------------------------------------------------- > > > > > > > > If I have a complex expression, I do not try to wrap it at predefined > > > > 80-column boundaries, but at logical boundaries within the expression to make > > > > it more readable (the brain can't find matching parentheses fast, so we can > > > > help it by aligning the code according to topmost terms in the expression). > > > > > > > > Example: > > > > if (unlikely(call_some_function(s, value) != RET > > > > _SUCCESS) || > > > > (var_1 == prev_var_1 && var_2 == prev_var_2) > > > > || > > > > flags & (FLAG_1 | FLAG_2) || > > > > some_other_condition) { > > > > } > > > > > > > > Now, if we impose 80-column limit, we get this. One may argue that is looks > > > > aesthetically better, but it is also less intelligible than the previous > > > > version: > > > > if (unlikely(call_some_function(s, value) != > > > > RET_SUCCESS) || (var_1 == prev_var_1 && > > > > var_2 == prev_var_2) || flags & (FLAG_1 | > > > > FLAG_2) || some_other_condition) { > > > > } > > > > > > > For starters, this is just crap. If you're writing code like this, then > > > line wrapping is really the least of your concerns. Take your function > > > return value and assign it to a variable before testing it in unlikely() > > > as per existing conventions and most of this goes away in this example. > > > > I wouldn't say that this is better: > > int csf_failed, vars_equal, flags_12; > > > > ... > > > > csf_failed = call_some_function(s, value) != RET_SUCCESS; > > vars_equal = var_1 == prev_var_1 && var_2 == prev_var_2; > > flags_12 = flags & (FLAG_1 | FLAG_2); > > if (unlikely(csf_failed) || vars_equal || > > flags_12 || some_other_conditions) { > > } > > > > If you think that it is better, it's OK, just write your code like that. > > And don't force it to everyone. > > > No, I wouldn't say that that's better either, but that's also not how I > suggested cleaning reworking it. We have existing conventions for how > complex blocks are broken down in to more readable forms which you seem > to have issues grasping. My point is that you are purposely obfuscating > things, and therefore your entire rationale is suspect at best. So, how would you clean this complex code block? Mikulas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/