Thread (7 messages) 7 messages, 4 authors, 2019-08-21

Re: [PATCH 2/2] drivers: input: mouse: alps: drop unneeded likely() call around IS_ERR()

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2019-08-21 17:49:02
Also in: lkml

Hi,

On Wed, Aug 21, 2019 at 01:37:09PM +0200, Enrico Weigelt, metux IT consult wrote:
On 20.08.19 16:22, Pali Rohár wrote:

Hi,
quoted
quoted
In that case, wouldn't a comment be more suitable for that ?
And why to add comment if current state of code is more-readable and
does not need it?
Readability is probably a bit subjective :p

With ongoing efforts of automatically identifying redundant code pathes,
the current situation causes the same discussion coming up over and over
again. Sooner or later somebody might get the idea to add a comment on
that line, that it's exactly as intented :o

OTOH, I'm unsure whether it's important to document that is particular
error path is unlikely, while we don't do it in thousands of other
places. IMHO, error pathes are supposed to be unlikely by nature,
otherwise we wouldn't call it an error situation ;-)
quoted
People normally add comments to code which is problematic to understand
or is somehow tricky, no so obvious or document how should code behave.
Yes, but isn't this case so obvious that it doesn't need any
documentation at all ? Is it so important to never ever forget that this
particular path is a rare situation ?
Because if I see "if (IS_ERR(...))" in an interrupt path I will try to
see if it can be optimized out, but in this particular case we document
it with explicit "unlikely" and I know that I do not need to bother.

The fact that there is unlikely in IS_ERR is an implementation detail.
It may be gone tomorrow. I do not want to have to remember all
implementation details of all kernel APIs and readjust the code all the
time as they are change underneath me.

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help