From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752265AbcFJAUQ (ORCPT ); Thu, 9 Jun 2016 20:20:16 -0400 Received: from out1134-218.mail.aliyun.com ([42.120.134.218]:9830 "EHLO out1134-218.mail.aliyun.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751573AbcFJAUM (ORCPT ); Thu, 9 Jun 2016 20:20:12 -0400 X-Alimail-AntiSpam: AC=CONTINUE;BC=0.0748889|-1;FP=0|0|0|0|0|-1|-1|-1;HT=e02c03303;MF=chengang@emindsoft.com.cn;NM=1;PH=DS;RN=10;RT=10;SR=0;TI=SMTPD_----4uapT56_1465517992; Message-ID: <575A0918.4030009@emindsoft.com.cn> Date: Fri, 10 Jun 2016 08:26:00 +0800 From: Chen Gang User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Rik van Riel , akpm@linux-foundation.org, trivial@kernel.org CC: vdavydov@virtuozzo.com, hannes@cmpxchg.org, mhocko@suse.cz, davem@davemloft.net, tj@kernel.org, linux-kernel@vger.kernel.org, Chen Gang Subject: Re: [PATCH trivial] include/linux/memcontrol.h: Clean up code only References: <1465485832-24175-1-git-send-email-chengang@emindsoft.com.cn> <1465493968.15275.3.camel@redhat.com> In-Reply-To: <1465493968.15275.3.camel@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6/10/16 01:39, Rik van Riel wrote: > On Thu, 2016-06-09 at 23:23 +0800, chengang@emindsoft.com.cn wrote: >> From: Chen Gang >> >> Merge several statements to one return statement, since the new >> return >> statement is still simple enough. > > This code is not simple, and any change that > makes it harder to read needs a good reason. > > At least in my opinion, your return statement > merging thing makes the code harder to read. > For me, every member has his/her own taste, we need talk about it case by case. [...] >> >> static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg) >> { >> - if (mem_cgroup_disabled()) >> - return 0; >> - >> - return memcg->css.id; >> + return mem_cgroup_disabled() ? 0 : memcg->css.id; >> } >> For me, this is the simplest way for using "? :", so it is easy enough ( I guess, Linux kernel does not completely reject "? :"). >> /** >> @@ -341,10 +338,7 @@ static inline unsigned short >> mem_cgroup_id(struct mem_cgroup *memcg) >> */ >> static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short >> id) >> { >> - struct cgroup_subsys_state *css; >> - >> - css = css_from_id(id, &memory_cgrp_subsys); >> - return mem_cgroup_from_css(css); >> + return mem_cgroup_from_css(css_from_id(id, &memory_cgrp_subsys)); >> } >> For me, this case may depend on various members' tastes (although it is simple enough to me -- it is in one line within 80 columns, and related with 2 functions' call). For this case, if any of another member suggests to keep original code no touch, too, I shall send patch v2 for it (keep it no touch). >> /** >> @@ -390,9 +384,7 @@ ino_t page_cgroup_ino(struct page *page); >> >> static inline bool mem_cgroup_online(struct mem_cgroup *memcg) >> { >> - if (mem_cgroup_disabled()) >> - return true; >> - return !!(memcg->css.flags & CSS_ONLINE); >> + return mem_cgroup_disabled() || (memcg->css.flags & CSS_ONLINE); >> } >> For me, this is almost the simplest way for using "||", we can find this case in include/linux (60+ cases for '||', and 150+ for '&&'), and the new return statement is in one line within 80 columns. The new return statement is not the simplest, but it is still simple enough (which should be better than original several lines statements). Thanks. -- Chen Gang (陈刚) Managing Natural Environments is the Duty of Human Beings.