On Fri, Jul 14, 2017 at 9:23 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
On Fri, Jul 14, 2017 at 12:21 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
NAK. This takes unintentionally insane code and turns it intentionally insane. Any non-zero return is considered an error.
The right fix is almost certainly to just return -EINVAL unconditionally.
Btw, this is why I hate compiler warning fix patch series. Even when they don't actually break the code (and sometimes they do that too), they can actually end up making the code worse.
I generally agree, and this is also why I held up sending patches for the -Wformat warnings until you brought those up. I also frequently send patches for recently introduced warnings, which tend to have a better chance of getting reviewed by the person that just introduced the code, to catch this kind of mistake in my patches.
I also regularly run into cases where I send a correct patch and find that another broken patch has been applied the following day ;-)
The *intent* of that code was to return zero for the CAP_SYS_ADMIN. But the code has never done that in its lifetime and nobody ever noticed, so clearly the code shouldn't even have tried.
Makes sense, yes. In this case, the review process has failed as well, as one of the maintainers even gave an Ack on the wrong patch, and then the patch got dropped without any feedback.
Arnd