linux-kernel - Re: [PATCH] mfd: asic3: One function call less in asic3_irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6e8eab5f-1f5c-b3dc-6b65-96a874ec2789@metux.net>
Date:   Mon, 8 Jul 2019 12:18:48 +0200
From:   "Enrico Weigelt, metux IT consult" <lkml@...ux.net>
To:     Markus Elfring <Markus.Elfring@....de>,
        Al Viro <viro@...iv.linux.org.uk>,
        kernel-janitors@...r.kernel.org
Cc:     Lee Jones <lee.jones@...aro.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mfd: asic3: One function call less in asic3_irq_probe()

On 07.07.19 09:56, Markus Elfring wrote:
>>> Avoid an extra function call by using a ternary operator instead of
>>> a conditional statement.
>>
>> Which is a good thing, because...?
> 
> I suggest to reduce a bit of duplicate source code also at this place.

Duplicate code (logic) or just characters ?

IMHO, readability is an important aspect, so we could be careful about
that. Some of your other patches IMHO made it actually a bit easier
to read, but this particular case doesnt seem so to (just according
to my personal taste).

I believe the compiler can do optimize that, based on the given flags.
(eg. size vs. speed). Therefore, I think that readability for the
human reader should be primary argument.

>> ... except that the result is not objectively better by any real criteria.
> 
> We can have different opinions about the criteria which are relevant here.

Which criterias are you operating on ?

> I dare to point another change possibility out.
> I am unsure if this adjustment will be picked up finally.

I think it's good that you're using tools like cocci for pointing out
*possible* points of useful refactoring. But that doesn't mean that a
particular patch can be accepted or not in the greater context.

Note that such issues are pretty subjective - it's not a technical but
an asthetic matter, so such issues can't be resolved by logic. Here, the
better something fits the personal taste of the maintainrs, the easier
it is for them to quickly understand the code (w/o having to give it any
deeper thoughts), thus reduces their brain load. Therefore that should
be the the primary argument left.

Don't see this as a judgment of your work as such - this kind of work
just tends to have a high rate of non-acceptable output (unless the
individual maintainer doing it himself).

--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@...ux.net -- +49-151-27565287