From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751391AbdKUUhp (ORCPT ); Tue, 21 Nov 2017 15:37:45 -0500 Received: from mout.web.de ([212.227.17.11]:56389 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184AbdKUUhn (ORCPT ); Tue, 21 Nov 2017 15:37:43 -0500 Subject: Re: [PATCH] cpupower: Combine two condition checks into one statement in get_cpu_topology() To: shuah@kernel.org, linux-pm@vger.kernel.org, Thomas Renninger Cc: LKML , kernel-janitors@vger.kernel.org, Shuah Khan References: <3daea226-c5cb-62e7-a025-8f758712a81a@users.sourceforge.net> <372aa4f8-7e97-ee83-1078-6f80e28ffdf0@kernel.org> From: SF Markus Elfring Message-ID: Date: Tue, 21 Nov 2017 21:37:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <372aa4f8-7e97-ee83-1078-6f80e28ffdf0@kernel.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:h9C8RUJlwSCFwSCkkCqEqFH4S1gkhZq7lxzpZzVzkA8SrrXULXl uiuyKQWBocXGwkztFjYvdWgW1jnERJvIFxELCs4kcdOWaoyf8I/X+Dwe1oSSgVi+keY1uZ5 BXG5x8YLO2Sb9thHuWJWUHTx1s0Y5hGVeoGXxWdCFyka3/ygezYpLz191tAePqSc+OAEKiX A+Onbjgn26Bmj+DFjBTmw== X-UI-Out-Filterresults: notjunk:1;V01:K0:8wTjBxnJlBk=:s3TQEQSSm7rDcp48VuZLp5 swcaCo8SnzPAmTMueSPNH3Vl34Y+7f/+bc72FUsdPboKuOTxynsCnRE5uHjHKxHtoRGGQJAZk jynRvsKE2wKeB9X/8r6VPe4AWg6PUPawTr+P1y7LfSNQ3M9BCG3RMOixqYATV5GqwNNklfYdl EkchgDXCC/Pe7ieubIRvsrgWNAEyehDwWWI3Tui6TuZP84DX8cZjVLw/wh0VgcWWx/lHWM0T/ 066kMsrzutl24x+6LRfQA32rZAeaLUFyIIdVt3rG/rszclaja8cr/2obEQtdk3csNkVrm0qEj J3ryMrCr6FIUKF8owj1dM0rsJwjtiH3Rl/jMCgLZftO55v668GiIxFC2Z2XjOanPXHaZ13Mjy 1gNtT8mLNyjjHlR/m/kN0Xm/Rh4aCyhBz1jUX89SVqmQMg30sRwvssMiQOmyS+dQQu4N+TC4s 0PeOsJ4rfELlQ1v035J3OWV742fsDmqladA0s94D8j9NMlBBlteuTTl2o4AAVZD8vseDhM4Gf pqn/B3D34YyCdak/cerjlkSSXmVKqKVn77aAXnDW7Nt5ytwDO+m+n8TQiigmlfVvi8c1+4MID Og5Ws4J/Bj/DNicc94+Wd9XNYBLT7NvyTpv+6YGCWUJWToDXMoYRJgBixkGeuU5gaDM4FQS6B xI1ijm5pVWQpj/eHv7+LzhUpe1CChPZzMH9TI2uCUvooVRZGlaS4is9y7BmTSP7BjW/+hUODA UmNsxKIX++M40sEJQRjbOuo2xlqqWb+0z5VdizdN9NtMWGuvAmHYysL0Fl/FRVsde9dEUPTG3 zq2xYufG/du0Fjqx7jKayJui43QnKAJ8zpd0PFy/iLW4W8n+V4= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> @@ -140,18 +140,12 @@ int get_cpu_topology(struct cpupower_topology *cpu_top) >> for (cpu = 0; cpu < cpus; cpu++) { >> cpu_top->core_info[cpu].cpu = cpu; >> cpu_top->core_info[cpu].is_online = cpupower_is_cpu_online(cpu); >> - if(sysfs_topology_read_file( >> - cpu, >> - "physical_package_id", >> - &(cpu_top->core_info[cpu].pkg)) < 0) { >> - cpu_top->core_info[cpu].pkg = -1; >> - cpu_top->core_info[cpu].core = -1; >> - continue; >> - } >> - if(sysfs_topology_read_file( >> - cpu, >> - "core_id", >> - &(cpu_top->core_info[cpu].core)) < 0) { >> + if (sysfs_topology_read_file(cpu, "physical_package_id", >> + &(cpu_top->core_info[cpu].pkg)) >> + < 0 || >> + sysfs_topology_read_file(cpu, "core_id", >> + &(cpu_top->core_info[cpu].core)) >> + < 0) { > > This change takes the easily readable code into hard to read code, I got other views around code readability for the shown software design direction. > even though it removes the duplicate code in the conditional. Would you like to reduce a bit of duplicate code here? > Please find a better way to make it not so hard to read. Which source code layout would you find more appropriate then in this use case? Regards, Markus