Originally posted by set135
Announcement
Collapse
No announcement yet.
Linux 5.15 Enabling "-Werror" By Default For All Kernel Builds
Collapse
X
-
Originally posted by indepe View PostI'd say fixing all warnings saves debugging time overall, in the long run.
I've seen significant amounts of time wasted, and bugs actually getting introduced, in the course of fixing false-positive warnings (i.e. those which didn't indicate any real problem). In many instances, the resulting code is worse, in terms of being less readable or maintainable. I would argue that most of these warnings shouldn't even have been enabled.
I've also seen developers take the easy way out. Such as getting rid of unused variables holding return code values, rather than actually checking them and handling errors appropriately. But hey, the warning count was reduced, so the code must be better!
- Likes 2
Comment
-
Originally posted by coder View PostDepends on what you mean by "all warnings". Some developers tend to be fairly aggressive, in terms of which they enable. Unfortunately, the more advanced warnings tend to be less reliable, which becomes problematic when someone decides to enact a -Werror policy.
There are the Diagnostic Pragmas in gcc clang and other compilers to selectively turn off the advance warnings in case of a false positive.
The problem here the worse of the more advance warning has less than a 10% false positive rate. So yes less reliable but is it justifed not to enabled it comes at the cost of letting the 90% of the detections when its right past past.
Yes it pain to code to selectively turn the advanced warning off when the compiler is truly wrong.
Code:#pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wuninitialized" foo(b); /* no diagnostic for this one */ #pragma GCC diagnostic pop
The case where it turns into horrible ruining code is when the person doing the repair does not see the problem as a false positive the cause of this is the fact over 90% of the time the advanced warning is right there is a problem and the code does need to be changed. Yes when the code does need to be changed because the error is real there is the equal problem
What you say next coder:
Originally posted by coder View PostI've seen significant amounts of time wasted, and bugs actually getting introduced, in the course of fixing false-positive warnings (i.e. those which didn't indicate any real problem). In many instances, the resulting code is worse, in terms of being less readable or maintainable. I would argue that most of these warnings shouldn't even have been enabled.
This is about having people doing the repairs know what the heck they are doing. Lot of people don't understand how little code is required to turn off advance warnings selectively. Yes there is a big problem with people not having the skill to work out that something is a false positive that should be a 3 line patch or less and should be reported to compiler so they can fix it.
Originally posted by coder View PostI've also seen developers take the easy way out. Such as getting rid of unused variables holding return code values, rather than actually checking them and handling errors appropriately. But hey, the warning count was reduced, so the code must be better!
__attribute__ ((warn_unused_result))
The fact a developer can do that is a different bug in the code. Yes warn_unused_result on the function means removing unused variable holding return values still equals warning/error. Yes MSVC has equal declare on functions as well.
We do need to get in the habit when coding to put proper attributes on functions so that function that return import error codes or results that must be freed cannot black holed causing future issues.
- Likes 3
Comment
-
I had a situation recently that surprised me at the time. The architecture I work on has 512 general purpose registers and the ability to address them like RAM. Not too surprisingly, the C compiler doesn't have any recognition of using pointers in this manner. It could auto allocate a variable to a register but all my attempts to define or reassign a pointer to general register space either got warnings or errors.
In the end I used inline assembly to eliminate any confusion.
Comment
-
Originally posted by coder View PostDepends on what you mean by "all warnings". Some developers tend to be fairly aggressive, in terms of which they enable. Unfortunately, the more advanced warnings tend to be less reliable, which becomes problematic when someone decides to enact a -Werror policy.
I've seen significant amounts of time wasted, and bugs actually getting introduced, in the course of fixing false-positive warnings (i.e. those which didn't indicate any real problem). In many instances, the resulting code is worse, in terms of being less readable or maintainable. I would argue that most of these warnings shouldn't even have been enabled.
I've also seen developers take the easy way out. Such as getting rid of unused variables holding return code values, rather than actually checking them and handling errors appropriately. But hey, the warning count was reduced, so the code must be better!
I tried write patch for many projects, where build system prints warning after warning and in the end, I couldn't even quickly see from build log if my code introduced any new warnings...
Yes, you can create wrong fixes, but I believe review process for Linux kernel is good enough to catch these wrong solutions before they go in.
Comment
-
Create a combination matrix of supported architectures/compilers.
It grows really fast. Werror prevented me from doing stupid things like assuming int size, or relying on arch specific overflow behaviour (BE/LE differences).
Imagine writing a new driver, testing it on amd64, works fine. Then somebody tries running it on a Big endian MIPS machine. What happens can be very, very surprising... these compiler warnings/errors help catch *a lot* of the low hanging fruit.
- Likes 2
Comment
-
Originally posted by coder View PostDepends on what you mean by "all warnings". Some developers tend to be fairly aggressive, in terms of which they enable.
Originally posted by coder View PostUnfortunately, the more advanced warnings tend to be less reliable, which becomes problematic when someone decides to enact a -Werror policy.
I've seen significant amounts of time wasted, and bugs actually getting introduced, in the course of fixing false-positive warnings (i.e. those which didn't indicate any real problem). In many instances, the resulting code is worse, in terms of being less readable or maintainable. I would argue that most of these warnings shouldn't even have been enabled.
- Likes 1
Comment
-
Originally posted by NobodyXu View PostLinux kernel doesn’t use standard IO functions provided by C stdio.h, so this option likely do nothing.
Code:extern int my_printf (void *my_object, const char *my_format, ...) __attribute__ ((format (printf, 2, 3)));
Yes surprise its already declared all over the place around the Linux kernel. So yes -Werror=format-security if you do turn it on with the Linux kernel it really does something.
- Likes 2
Comment
Comment