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