lkml.org 
[lkml]   [2019]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] mfd: asic3: One function call less in asic3_irq_probe()
From
Date
>> 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.


>> This issue was detected by using the Coccinelle software.
>
> Oh, I see - that answers all questions.

Obviously not so far.


> "Software has detected an issue", so of course an issue it is.

The mentioned development tool can help to point refactoring
possibilities out.


>> - if (irq < asic->irq_base + ASIC3_NUM_GPIOS)
>> - irq_set_chip(irq, &asic3_gpio_irq_chip);
>> - else
>> - irq_set_chip(irq, &asic3_irq_chip);
>> -
>> + irq_set_chip(irq,
>> + (irq < asic->irq_base + ASIC3_NUM_GPIOS)
>> + ? &asic3_gpio_irq_chip
>> + : &asic3_irq_chip);
>
> ... except that the result is not objectively better by any real criteria.

We can have different opinions about the criteria which are relevant here.


> It's not more readable,

This is a possible view.


> it conveys _less_ information to reader

I guess that the interpretation of this feedback can become more interesting.


> (the fact that calls differ only by the last argument
> had been visually obvious already,

Can the repeated code specification make the recognition of this
implementation detail a bit harder actually?


> had been visually obvious already, and logics used to be easier
> to see), it (obviously) does not generate better (or different) code.

The functionality should be equivalent for the shown software refactoring.


> What the hell is the point?

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

Regards,
Markus

\
 
 \ /
  Last update: 2019-07-07 09:57    [W:3.858 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site