Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-21 Thread Miguel Ojeda
On Mon, Dec 21, 2020 at 11:02 AM Guenter Roeck wrote: > > On 12/20/20 10:18 PM, Masahiro Yamada wrote: > With a change like this, I'd have expected that there is a coccinelle > script or similar to ensure that claims made in the commit message > are true. It is only a warning -- the compiler alre

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-21 Thread Miguel Ojeda
On Mon, Dec 21, 2020 at 7:20 AM Masahiro Yamada wrote: > > Sorry for the delay. No problem! > Now I sent out the fix for lantiq_etop.c > > https://lore.kernel.org/patchwork/patch/1355595/ I saw it, thanks for the Cc! > The reason of the complication was > I was trying to merge the following pa

Re: [PATCH] net: lantiq_etop: check the result of request_irq()

2020-12-21 Thread Miguel Ojeda
On Mon, Dec 21, 2020 at 6:01 PM Andrew Lunn wrote: > > So please leave the warning in place, and maybe somebody else will > fully fix it. For context: the plan is to enable the warning unconditionally starting with 5.11. After that, the idea is making it an error as soon as reasonable (e.g. 5.12

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-13 Thread Miguel Ojeda
On Sun, Dec 13, 2020 at 4:38 PM 'Matthias Urlichs' via Clang Built Linux wrote: > > If your change to a function breaks its callers, it's your job to fix No function has changed. This patch enables a warning (that for some reason is an error in the case of Guenter). Even if this was a hard error

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-13 Thread Miguel Ojeda
On Sun, Dec 13, 2020 at 4:16 PM Greg KH wrote: > > Because if you get a report of something breaking for your change, you > need to work to resolve it, not argue about it. Otherwise it needs to > be dropped/reverted. Nobody has argued that. In fact, I explicitly said the opposite: "So I think we

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-13 Thread Miguel Ojeda
On Sun, Dec 13, 2020 at 1:55 PM Guenter Roeck wrote: > > Witz komm raus, Du bist umzingelt. Please, explain this reference. :-) > The key here is "if nobody complains". I would argue that it is _your_ > responsibility to do those builds, and not the reponsibility of others > to do it for you. T

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-12 Thread Miguel Ojeda
On Sat, Dec 12, 2020 at 5:18 PM Guenter Roeck wrote: > > This patch results in: > > arch/sh/kernel/cpu/sh4a/smp-shx3.c: In function 'shx3_prepare_cpus': > arch/sh/kernel/cpu/sh4a/smp-shx3.c:76:3: error: ignoring return value of > 'request_irq' declared with attribute 'warn_unused_result' > > when

Re: [PATCH v3] Compiler Attributes: remove CONFIG_ENABLE_MUST_CHECK

2020-12-02 Thread Miguel Ojeda
On Sat, Nov 28, 2020 at 8:34 PM Masahiro Yamada wrote: > > Revert commit cebc04ba9aeb ("add CONFIG_ENABLE_MUST_CHECK"). > > A lot of warn_unused_result warnings existed in 2006, but until now > they have been fixed thanks to people doing allmodconfig tests. > > Our goal is to always enable __must_

Re: [PATCH] compiler_attribute: remove CONFIG_ENABLE_MUST_CHECK

2020-11-27 Thread Miguel Ojeda
On Sat, Nov 21, 2020 at 8:44 PM Masahiro Yamada wrote: > > Revert commit cebc04ba9aeb ("add CONFIG_ENABLE_MUST_CHECK"). > > A lot of warn_unused_result warnings existed in 2006, but until now > they have been fixed thanks to people doing allmodconfig tests. > > Our goal is to always enable __must_

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-26 Thread Miguel Ojeda
On Thu, Nov 26, 2020 at 4:28 PM Geert Uytterhoeven wrote: > > The maintainer is not necessarily the owner/author of the code, and > thus may not know the intent of the code. Agreed, I was not blaming maintainers -- just trying to point out that the problem is there :-) In those cases, it is stil

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-26 Thread Miguel Ojeda
On Wed, Nov 25, 2020 at 11:44 PM Edward Cree wrote: > > To make the intent clear, you have to first be certain that you > understand the intent; otherwise by adding either a break or a > fallthrough to suppress the warning you are just destroying the > information that "the intent of this code

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-25 Thread Miguel Ojeda
On Wed, Nov 25, 2020 at 5:24 PM Jakub Kicinski wrote: > > And just to spell it out, > > case ENUM_VALUE1: > bla(); > break; > case ENUM_VALUE2: > bla(); > default: > break; > > is a fairly idiomatic way of indicating that not all values of the enum > are expected to

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-24 Thread Miguel Ojeda
On Wed, Nov 25, 2020 at 12:53 AM Finn Thain wrote: > > I'm saying that supporting the official language spec makes more sense > than attempting to support a multitude of divergent interpretations of the > spec (i.e. gcc, clang, coverity etc.) Making the kernel strictly conforming is a ship that s

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-24 Thread Miguel Ojeda
On Mon, Nov 23, 2020 at 9:38 PM James Bottomley wrote: > > So you think a one line patch should take one minute to produce ... I > really don't think that's grounded in reality. No, I have not said that. Please don't put words in my mouth (again). I have said *authoring* lines of *this* kind tak

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-24 Thread Miguel Ojeda
On Tue, Nov 24, 2020 at 1:58 AM Finn Thain wrote: > > What I meant was that you've used pessimism as if it was fact. "future mistakes that it might prevent" is neither pessimism nor states a fact. > For example, "There is no way to guess what the effect would be if the > compiler trained program

Re: [Intel-wired-lan] [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-24 Thread Miguel Ojeda
On Tue, Nov 24, 2020 at 11:24 PM Finn Thain wrote: > > These statements are not "missing" unless you presume that code written > before the latest de facto language spec was written should somehow be > held to that spec. There is no "language spec" the kernel adheres to. Even if it did, kernel co

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-23 Thread Miguel Ojeda
On Mon, Nov 23, 2020 at 4:58 PM James Bottomley wrote: > > Well, I used git. It says that as of today in Linus' tree we have 889 > patches related to fall throughs and the first series went in in > october 2017 ... ignoring a couple of outliers back to February. I can see ~10k insertions over ~1

Re: [PATCH] compiler_attribute: remove CONFIG_ENABLE_MUST_CHECK

2020-11-23 Thread Miguel Ojeda
On Mon, Nov 23, 2020 at 4:37 AM Masahiro Yamada wrote: > > I can move it to compiler_attribute.h > > This attribute is supported by gcc, clang, and icc. > https://godbolt.org/z/ehd6so > > I can send v2. > > I do not mind renaming, but it should be done in a separate patch. Of course -- sorry, I d

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-23 Thread Miguel Ojeda
On Sun, Nov 22, 2020 at 11:36 PM James Bottomley wrote: > > Well, it seems to be three years of someone's time plus the maintainer > review time and series disruption of nearly a thousand patches. Let's > be conservative and assume the producer worked about 30% on the series > and it takes about

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-23 Thread Miguel Ojeda
On Sun, Nov 22, 2020 at 11:54 PM Finn Thain wrote: > > We should also take into account optimisim about future improvements in > tooling. Not sure what you mean here. There is no reliable way to guess what the intention was with a missing fallthrough, even if you parsed whitespace and indentation

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-22 Thread Miguel Ojeda
On Sun, Nov 22, 2020 at 7:22 PM James Bottomley wrote: > > Well, it's a problem in an error leg, sure, but it's not a really > compelling reason for a 141 patch series, is it? All that fixing this > error will do is get the driver to print "oh dear there's a problem" > under four more conditions

Re: [PATCH] compiler_attribute: remove CONFIG_ENABLE_MUST_CHECK

2020-11-21 Thread Miguel Ojeda
On Sat, Nov 21, 2020 at 8:44 PM Masahiro Yamada wrote: > > Our goal is to always enable __must_check where appreciate, so this > CONFIG option is no longer needed. This would be great. It also implies we can then move it to `compiler_attributes.h` since it does not depend on config options anymor

Re: [PATCH 000/141] Fix fall-through warnings for Clang

2020-11-20 Thread Miguel Ojeda
Hi Gustavo, On Fri, Nov 20, 2020 at 7:21 PM Gustavo A. R. Silva wrote: > > Hi all, > > This series aims to fix almost all remaining fall-through warnings in > order to enable -Wimplicit-fallthrough for Clang. Thanks for this. Since this warning is reliable in both/all compilers and we are event

Re: [PATCH] compiler-clang: remove version check for BPF Tracing

2020-11-04 Thread Miguel Ojeda
; clang sources. Acked-by: Miguel Ojeda Cheers, Miguel

Re: [PATCH] tools/perf: Remove broken __no_tail_call attribute

2020-10-28 Thread Miguel Ojeda
On Wed, Oct 28, 2020 at 9:11 AM Peter Zijlstra wrote: > > Subject: tools/perf: Remove broken __no_tail_call attribute Acked-by: Miguel Ojeda Cheers, Miguel

Re: [PATCH net-next v2 3/6] bonding: rename slave to port where possible

2020-10-02 Thread Miguel Ojeda
Hi Jarod, On Fri, Oct 2, 2020 at 7:44 PM Jarod Wilson wrote: > > .clang-format |4 +-> #ifdef > CONFIG_NET_POLL_CONTROLLER Acked-by: Miguel Ojeda Cheers, Miguel

Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage

2020-06-04 Thread Miguel Ojeda
On Thu, Jun 4, 2020 at 4:56 PM Kees Cook wrote: > > Er? That's not what it looked like to me: > > #define IS_BUILTIN(option) __is_defined(option) > #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option)) > > But just to be sure, I just tested in with a real build: > > [3.242160]

Re: [PATCH 01/10] x86/mm/numa: Remove uninitialized_var() usage

2020-06-04 Thread Miguel Ojeda
On Thu, Jun 4, 2020 at 9:58 AM Thomas Gleixner wrote: > > but if we ever lose the 1 then the above will silently compile the code > within the IS_ENABLED() section out. Yeah, I believe `IS_ENABLED()` is only meant for Kconfig symbols, not macro defs in general. A better option would be `__is_defi

Re: [PATCH 10/10] compiler: Remove uninitialized_var() macro

2020-06-03 Thread Miguel Ojeda
igned-off-by: Kees Cook > --- +1, one less trick split between `compiler*` files. Reviewed-by: Miguel Ojeda Cheers, Miguel

Re: [PATCH 0/4] treewide: Add 'fallthrough' pseudo-keyword

2019-10-11 Thread Miguel Ojeda
Hi Linus, On Fri, Oct 11, 2019 at 6:30 PM Linus Torvalds wrote: > > On Sat, Oct 5, 2019 at 9:46 AM Joe Perches wrote: > > > > Add 'fallthrough' pseudo-keyword to enable the removal of comments > > like '/* fallthrough */'. > > I applied patches 1-3 to my tree just to make it easier for people to

Re: [PATCH 00/16] treewide: prefer __section from compiler_attributes.h

2019-08-13 Thread Miguel Ojeda
On Mon, Aug 12, 2019 at 11:53 PM Nick Desaulniers wrote: > > GCC unescapes escaped string section names while Clang does not. Because > __section uses the `#` stringification operator for the section name, it > doesn't need to be escaped. Thanks a lot Nick, this takes a weight off my mind. One __

Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR

2019-01-20 Thread Miguel Ojeda
On Sun, Jan 20, 2019 at 3:43 PM Michael S. Tsirkin wrote: > > No not yet. Sorry! Pls send this one in, barrier_data will likely miss > the next merge window. No worries! Done. Cheers, Miguel

Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR

2019-01-19 Thread Miguel Ojeda
Hi Michael, On Wed, Jan 9, 2019 at 3:50 PM Michael S. Tsirkin wrote: > > On Wed, Jan 09, 2019 at 11:35:52AM +0100, Miguel Ojeda wrote: > > Note it would be nice to separate the patch into two (one for the > > comments, another for OPTIMIZER_HIDE_VAR), and also possib

Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__

2019-01-19 Thread Miguel Ojeda
Hi Greg, Nick, Xiaozhou, On Thu, Dec 6, 2018 at 1:50 PM Greg KH wrote: > > If something is fixed in Linus's tree for this, I want to take it into > the 4.19-stable tree as well. This ended up in Linus' tree before the holidays, i.e. 4.20 has it, see commit 71391bdd2e9a ("include/linux/compiler_t

Re: [PATCH] iavf: Use printf instead of gnu_printf for iavf_debug_d

2019-01-16 Thread Miguel Ojeda
We can convert from gnu_printf to printf without any side effects for > two reasons: > > 1. All iavf_debug instances use standard printf formats, as pointed out >by Miguel Ojeda at the below link, meaning gnu_printf is not strictly >required. > > 2. However, GCC has a

Re: [PATCH RFC 1/4] include/linux/compiler*.h: fix OPTIMIZER_HIDE_VAR

2019-01-09 Thread Miguel Ojeda
On Tue, Jan 8, 2019 at 6:44 PM Nick Desaulniers wrote: > > Also for more context, see: > commit 7829fb09a2b4 ("lib: make memzero_explicit more robust against > dead store elimination") By the way, shouldn't that barrier_data() be directly in compiler.h too, since it is for both gcc & clang? > Re

Re: [PATCH] Compiler Attributes: move kernel-only attributes into __KERNEL__

2018-11-29 Thread Miguel Ojeda
On Thu, Nov 29, 2018 at 3:16 AM Xiaozhou Liu wrote: > > On Wed, Nov 28, 2018 at 06:35:18PM +0100, Miguel Ojeda wrote: > > By `these' I mean inline and the like, to be clear. Ah, that makes more sense! Sorry. > > That is not exactly correct -- a3f8a30f3f00 moved some at

Re: [PATCH] net: phy: TLK10X initial driver submission

2018-04-19 Thread Miguel Ojeda
On Thu, Apr 19, 2018 at 10:28 AM, Måns Andersson wrote: > From: Mans Andersson > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > Hi Mans, Some quick notes. > In addition the TLK10X needs to be removed from DP83848 driver as the > power back off support is added here for t

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

2018-03-16 Thread Miguel Ojeda
On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds wrote: > On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda > wrote: >>> >>> Kees - is there some online "gcc-4.4 checker" somewhere? This does >>> seem to work with my gcc. I actually tested some of those fil

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

2018-03-16 Thread Miguel Ojeda
On Fri, Mar 16, 2018 at 9:14 PM, Linus Torvalds wrote: > On Fri, Mar 16, 2018 at 1:03 PM, Miguel Ojeda > wrote: >>> >>> Kees - is there some online "gcc-4.4 checker" somewhere? This does >>> seem to work with my gcc. I actually tested some of those fil

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

2018-03-16 Thread Miguel Ojeda
On Fri, Mar 16, 2018 at 8:27 PM, Linus Torvalds wrote: > On Fri, Mar 16, 2018 at 10:55 AM, Al Viro wrote: >> >> That's not them, that's C standard regarding ICE. > > Yes. The C standard talks about "integer constant expression". I know. > It's come up in this very thread before. > > The C standar

Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal

2018-03-15 Thread Miguel Ojeda
On Fri, Mar 16, 2018 at 12:49 AM, Kees Cook wrote: > On Thu, Mar 15, 2018 at 4:46 PM, Linus Torvalds > wrote: >> What I'm *not* so much ok with is "const_max(5,sizeof(x))" erroring >> out, or silently causing insane behavior due to hidden subtle type >> casts.. > > Yup! I like it as an explicit a

Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal

2018-03-15 Thread Miguel Ojeda
On Fri, Mar 16, 2018 at 12:08 AM, Miguel Ojeda wrote: > On Thu, Mar 15, 2018 at 11:58 PM, Miguel Ojeda > wrote: >> On Thu, Mar 15, 2018 at 11:46 PM, Kees Cook wrote: >>> >>> By using this eye-bleed: >>> >>> size_t __error_not_const_arg(void) \ &g

Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal

2018-03-15 Thread Miguel Ojeda
On Thu, Mar 15, 2018 at 11:58 PM, Miguel Ojeda wrote: > On Thu, Mar 15, 2018 at 11:46 PM, Kees Cook wrote: >> >> By using this eye-bleed: >> >> size_t __error_not_const_arg(void) \ >> __compiletime_error("const_max() used with non-compile-time constant arg

Re: [PATCH v4 1/2] kernel.h: Introduce const_max() for VLA removal

2018-03-15 Thread Miguel Ojeda
On Thu, Mar 15, 2018 at 11:46 PM, Kees Cook wrote: > On Thu, Mar 15, 2018 at 3:23 PM, Linus Torvalds > wrote: >> On Thu, Mar 15, 2018 at 3:16 PM, Kees Cook wrote: >>> >>> size_t __error_not_const_arg(void) \ >>> __compiletime_error("const_max() used with non-compile-time constant arg"); >>> #def

Re: [PATCH] drivers: net: wireless: ath: ath9: dfs: remove VLA usage

2018-03-13 Thread Miguel Ojeda
On Sun, Mar 11, 2018 at 12:12 AM, Kees Cook wrote: > > The problem is that it's not a "constant expression", so the compiler > frontend still yells about it under -Wvla. I would characterize this > mainly as a fix for "accidental VLA" or "misdetected VLA" or something > like that. AIUI, there real

Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()

2018-03-10 Thread Miguel Ojeda
On Sat, Mar 10, 2018 at 6:51 PM, Linus Torvalds wrote: > > So in *historical* context - when a compiler didn't do variable length > arrays at all - the original semantics of C "constant expressions" > actually make a ton of sense. > > You can basically think of a constant expression as something t

Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()

2018-03-10 Thread Miguel Ojeda
On Sat, Mar 10, 2018 at 5:30 PM, Linus Torvalds wrote: > On Sat, Mar 10, 2018 at 7:33 AM, Kees Cook wrote: >> >> Alright, I'm giving up on fixing max(). I'll go back to STACK_MAX() or >> some other name for the simple macro. Bleh. > > Oh, and I'm starting to see the real problem. > > It's not tha

Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()

2018-03-09 Thread Miguel Ojeda
On Sat, Mar 10, 2018 at 7:10 AM, Miguel Ojeda wrote: > On Sat, Mar 10, 2018 at 4:11 AM, Randy Dunlap wrote: >> On 03/09/2018 04:07 PM, Andrew Morton wrote: >>> On Fri, 9 Mar 2018 12:05:36 -0800 Kees Cook wrote: >>> >>>> When max() is used in stack arra

Re: [PATCH v3] kernel.h: Skip single-eval logic on literals in min()/max()

2018-03-09 Thread Miguel Ojeda
On Sat, Mar 10, 2018 at 4:11 AM, Randy Dunlap wrote: > On 03/09/2018 04:07 PM, Andrew Morton wrote: >> On Fri, 9 Mar 2018 12:05:36 -0800 Kees Cook wrote: >> >>> When max() is used in stack array size calculations from literal values >>> (e.g. "char foo[max(sizeof(struct1), sizeof(struct2))]", the