From: Michal Nazarewicz <mina86@mina86.com>
To: Michael Lentine <mlentine@google.com>
Cc: Ingo Molnar <mingo@elte.hu>, linux-kernel@vger.kernel.org,
        Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH] tools: perf: prefer clarity in setup_pager
In-Reply-To: <CAB1dqC7BqsaDtXVYx24M5R9eaY7ZfpEtu3bxbJHXNHYhAbJETg@mail.gmail.com>
Organization: http://mina86.com/
References: <1404668531-31550-1-git-send-email-mina86@mina86.com> <CAB1dqC7BqsaDtXVYx24M5R9eaY7ZfpEtu3bxbJHXNHYhAbJETg@mail.gmail.com>
User-Agent: Notmuch/0.17+15~gb65ca8e (http://notmuchmail.org) Emacs/24.4.50.1 (x86_64-unknown-linux-gnu)
Face: iVBORw0KGgoAAAANSUhEUgAAADAAAAAwBAMAAAClLOS0AAAAJFBMVEWbfGlUPDDHgE57V0jUupKjgIObY0PLrom9mH4dFRK4gmjPs41MxjOgAAACQElEQVQ4jW3TMWvbQBQHcBk1xE6WyALX1069oZBMlq+ouUwpEQQ6uRjttkWP4CmBgGM0BQLBdPFZYPsyFUo6uEtKDQ7oy/U96XR2Ux8ehH/89Z6enqxBcS7Lg81jmSuujrfCZcLI/TYYvbGj+jbgFpHJ/bqQAUISj8iLyu4LuFHJTosxsucO4jSDNE0Hq3hwK/ceQ5sx97b8LcUDsILfk+ovHkOIsMbBfg43VuQ5Ln9YAGCkUdKJoXR9EclFBhixy3EGVz1K6eEkhxCAkeMMnqoAhAKwhoUJkDrCqvbecaYINlFKSRS1i12VKH1XpUd4qxL876EkMcDvHj3s5RBajHHMlA5iK32e0C7VgG0RlzFPvoYHZLRmAC0BmNcBruhkE0KsMsbEc62ZwUJDxWUdMsMhVqovoT96i/DnX/ASvz/6hbCabELLk/6FF/8PNpPCGqcZTGFcBhhAaZZDbQPaAB3+KrWWy2XgbYDNIinkdWAFcCpraDE/knwe5DBqGmgzESl1p2E4MWAz0VUPgYYzmfWb9yS4vCvgsxJriNTHoIBz5YteBvg+VGISQWUqhMiByPIPpygeDBE6elD973xWwKkEiHZAHKjhuPsFnBuArrzxtakRcISv+XMIPl4aGBUJm8Emk7qBYU8IlgNEIpiJhk/No24jHwkKTFHDWfPniR4iw5vJaw2nzSjfq2zffcE/GDjRC2dn0J0XwPAbDL84TvaFCJEU4Oml9pRyEUhR3Cl2t01AoEjRbs0sYugp14/4X5n4pU4EHHnMAAAAAElFTkSuQmCC
Date: Mon, 07 Jul 2014 14:32:08 +0200
Message-ID: <xa1tmwcl1h9z.fsf@mina86.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Sender: linux-kernel-owner@vger.kernel.org

On Sun, Jul 06 2014, Michael Lentine <mlentine@google.com> wrote:
> Unless I'm missing something this removes defaulting the pager to cat
> if nothing is found which is very useful for devices/oses without less
> or pager.

The last two conditions are merged together, i.e.

       if (!pager)
               pager = "cat";
       if (!*pager || !strcmp(pager, "cat"))
               return;

becomes:

       if (!pager || !*pager || !strcmp(pager, "cat"))
               return;

This also in my opinion simplifies the code since the original is a bit
convoluted.  If pager is NULL it first assigns "cat" to it and then in
the very next condition compares pager to "cat".  With this change, it's
more obvious that if pager is NULL, the function will terminate.

> On Sun, Jul 6, 2014 at 10:42 AM, Michal Nazarewicz <mina86@mina86.com>
> wrote:
>> “!(pager || access(…))” is indeed pretty smart way to write
>> “!pager && access(…) == 0” but other than being clever it gives
>> no advantages and merely confuses the reader who needs to wonder
>> what is actually going on.
>>
>> As such, replace the checks with much cleaner ones.
>>
>> Also, while at it, merge the lest “!pager” test with the next
>> test that yields true after the “!pager” if's body is executed.
>> ---
>>  tools/perf/util/pager.c | 8 +++-----
>>  1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/perf/util/pager.c b/tools/perf/util/pager.c
>> index 31ee02d..14da1b0 100644
>> --- a/tools/perf/util/pager.c
>> +++ b/tools/perf/util/pager.c
>> @@ -57,13 +57,11 @@ void setup_pager(void)
>>         }
>>         if (!pager)
>>                 pager = getenv("PAGER");
>> -       if (!(pager || access("/usr/bin/pager", X_OK)))
>> +       if (!pager && access("/usr/bin/pager", X_OK) == 0)
>>                 pager = "/usr/bin/pager";
>> -       if (!(pager || access("/usr/bin/less", X_OK)))
>> +       if (!pager && access("/usr/bin/less", X_OK) == 0)
>>                 pager = "/usr/bin/less";
>> -       if (!pager)
>> -               pager = "cat";
>> -       if (!*pager || !strcmp(pager, "cat"))
>> +       if (!pager || !*pager || !strcmp(pager, "cat"))
>>                 return;
>>
>>         spawned_pager = 1; /* means we are emitting to terminal */

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@google.com>--<xmpp:mina86@jabber.org>--ooO--(_)--Ooo--
--
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/