I might have sounded harsh but I think shit happens is not the way to look at this. Don't claim I'm a better developer, but I always try to shy away from making things look nicer.
Experience have thought me, deal with problems when it is a problem. Dealing with could be problems can be a deep, very deep rabbit hole.
The commit message gave me the feeling that we should have just trust the author.
> I always try to shy away from making things look nicer
That's understandable, though from my experience, lots of old bugs can be found while refactoring code, even at the (small) risk of introducing new bugs.
As (almost) always, the expert's answer is: "It depends". How risky is the change, how big the consequences, how un-nice is the code before, how easy is it to test that the code still works afterwards, etc...
FWIW, I tend to err on the side of "do it", and I usually do it. But I have been in a situation where a customer asked for the risk level, I answered to the best of my knowledge (quite low but it's hard to be 100% sure), and they declined the change. The consequences of a bug would have been pretty horrible, too. Hundreds of thousands of (things) shipped with buggy software that is somewhat cumbersome to update.
While true, it's important to ensure there is adequate test coverage before trying to refactor, in case you miss something.
Also, try to avoid small commits / changes; churn in code should be avoided, especially in kernel code. IIRC the Linux project and a lot of open source projects do not accept 'refactoring' pull requests, among other things for this exact reason.
Agree, but even 100% test coverage can't catch this kind of bug. I don't know of any systematic testing method which would be able to catch it. Maybe something like valgrind which detects accesses to uninitialized memory, but then you'd still have to execute very special code paths (which is "more" than 100% coverage).
A lot of times, this is just shifting the problem to the future and making life harder.
We have a team like this -- their processes often failing, and their error reporting is lacking in important details. But they are not willing to improve reporting / make errors nicer (=with relevant details), instead they have to manually dig into the logs to see what happens. They waste a lot of time because they "shy away from making things look nicer."
Caveat - I know this doesn't directly apply to the vulnerability at hand, but is a discussion of a tangential view.
> Experience have thought me, deal with problems when it is a problem.
Experience has taught me that disparate ways of doing the same thing tend to have bugs in one or more of the implementations. Then trying to figure out if a specific bug exists other places requires digging into those other places.
Make it work. Make it good. Make it faster (as necessary) is the way my long-lived code tends to evolve.
Nonsense. It's just easy to blame refactoring when it breaks something. "You fool! Why did you change things? It was perfectly fine before.". Much harder to say "Why has this bug been here for 10 years? Why did nobody refactor the code?" even when it would have helped.
Not refactoring code also sacrifices long term issues in return for short term risk reduction. Look at all of the government systems stuck on COBOL. I guarantee there was someone in the 90s offering to rewrite it in Java, and someone else saying "no it's too risky!". Then your ancient system crashes in 2022 and nobody knows how it works let alone how to fix it.
Experience have thought me, deal with problems when it is a problem. Dealing with could be problems can be a deep, very deep rabbit hole.
The commit message gave me the feeling that we should have just trust the author.
https://github.com/torvalds/linux/commit/f6dd975583bd8ce0884...