On Sun, 9 Feb 2025 at 10:45, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
>
> Again, lets look eat_empty_buffer().
>
> The comment says "maybe it's empty" but how/why can this happen ?
WHY DO YOU CARE?
Even if it's just defensive programming, it's just plain good code,
when the rule is "the caller is waiting for data".
And if it means that you don't need to add random WARN_ON_ONCE() statements.
So here's the deal: either you
(a) convince yourself that the length really can never be true, and
you write a comment about why that is the case.
or
(b) you DON'T convince yourself that that is true, and you leave
eat_empty_buffer() alone.
and both of those situations are fine.
But adding a random sprinkling of WARN_ON_ONCE() statements is worse
than *either* of those two cases.
See what my argument is? Adding a WARN_ON is just making the code
worse. Don't do it. It's the worst of both worlds: it adds code to
generate, and it adds confusion to readers ("why are we warning?" or
"when can this happen").
In contrast, the "eat_empty_buffer()" case just saying "if it's an
empty buffer, it doesn't satisfy my needs, so I'll just release the
empty buffer and go on".
That's simple and straightforward. Easy to maintain, and more
efficient than your WARN_ONs.
And if you can convince yourself it's not needed, you remove it.
EXACTLY LIKE THE WARN_ON.
So there's simply never any upside to the WARN_ON.
Linus
