From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753487AbdK3AVY (ORCPT ); Wed, 29 Nov 2017 19:21:24 -0500 Received: from mail-vk0-f68.google.com ([209.85.213.68]:44038 "EHLO mail-vk0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753339AbdK3AVW (ORCPT ); Wed, 29 Nov 2017 19:21:22 -0500 X-Google-Smtp-Source: AGs4zMZOrJ7OIS7ZC5MEnNG4Vnb54uO66Z/16l5Npc5SCHnQRSzIjzxWLEfIRXbuyXJ9oFjvcqq9+pFzXT64IVmvhvc= MIME-Version: 1.0 In-Reply-To: References: <20171127235253.GA20384@embeddedor.com> <20171128120512.Horde.1mz61Up1PsNtyHbrjWmK8L7@gator4166.hostgator.com> <20171128122235.Horde.vFP-9ZfAP0f9BFNePB8Z8xi@gator4166.hostgator.com> <20171128190032.2b1fa464@alans-desktop> <20171128142532.Horde.i2oBtHDOaD7XV1M3yAL7rga@gator4166.hostgator.com> <20171129091050.Horde.2q2U63NpkxZnJ2HEQ6hKGaB@gator4166.hostgator.com> From: Kees Cook Date: Wed, 29 Nov 2017 16:21:20 -0800 X-Google-Sender-Auth: zkWEw-qnzSkz3uYhnj-2LyoPOzc Message-ID: Subject: Re: [PATCH] x86/syscalls: Mark expected switch fall-throughs To: Thomas Gleixner Cc: "Gustavo A. R. Silva" , Alan Cox , Ingo Molnar , "H. Peter Anvin" , X86 ML , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 29, 2017 at 7:14 AM, Thomas Gleixner wrote: > On Wed, 29 Nov 2017, Gustavo A. R. Silva wrote: >> Quoting Thomas Gleixner : >> >> > >> > So I have to ask WHY this information was not in the changelog of the patch >> > in question: >> > >> > 1) How it works >> > >> > 2) Why comments have been chosen over macros >> > >> >> I will add this info and send the patch again. >> >> > > In preparation to enabling -Wimplicit-fallthrough, mark switch cases >> > > where we are expecting to fall through. >> > >> > It's not a reviewers job to chase that information down. >> > >> > While I can understand that the comments are intentional due to existing >> > tools, I still prefer the macro/annotation. But I'm not religious about it >> > when there is common consensus. :) > > ^^^^^^^^^^^^^^^^ > > This is the important point. And there are people aside of me who prefer the > macro annotation. Understood. I, too, prefer the macro annotation, but when considering where the primary benefit comes from, I had to admit that using the comments was tolerable: the many external tools (e.g. Coverity, Eclipse, gcc, clang, etc) that already process the comments means we gain their coverage without any additional work to teach them about yet another way to mark fall-through. In other words, making a marking that only works for humans and gcc leaves out the other tools. To me, "it's ugly" isn't sufficient to limit the wider benefit. And I do recognize that when we get all fixes landed and we add -Wimplicit-fallthrough, then we can just ignore the tools that don't understand the new marking and announce false positives: but it's not worth everyone else's time to deal with those false positives. There is already a solution that all "missing break" tools handle, so let's use it, all false positives vanish, and everyone wins (with a small "ugliness" cost). -Kees -- Kees Cook Pixel Security