Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751130AbdGNU2c (ORCPT ); Fri, 14 Jul 2017 16:28:32 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:34674 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751012AbdGNU2a (ORCPT ); Fri, 14 Jul 2017 16:28:30 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170714092540.1217397-1-arnd@arndb.de> <20170714092540.1217397-4-arnd@arndb.de> From: Arnd Bergmann Date: Fri, 14 Jul 2017 22:28:29 +0200 X-Google-Sender-Auth: fdJlNifHKzTHr6tIkYYcZ9xKWww Message-ID: Subject: Re: [PATCH, RESEND 03/14] drm/vmwgfx: avoid gcc-7 parentheses warning To: Linus Torvalds Cc: Linux Kernel Mailing List , VMware Graphics , Sinclair Yeh , Thomas Hellstrom , David Airlie , Greg Kroah-Hartman , Tejun Heo , Guenter Roeck , IDE-ML , Linux Media Mailing List , Andrew Morton , DRI , Brian Paul Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1450 Lines: 32 On Fri, Jul 14, 2017 at 9:23 PM, Linus Torvalds wrote: > On Fri, Jul 14, 2017 at 12:21 PM, Linus Torvalds > 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