Re: [PATCH, RESEND 03/14] drm/vmwgfx: avoid gcc-7 parentheses warning

From: Sinclair Yeh <syeh@vmware.com>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Thomas Hellstrom <thellstrom@vmware.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	DRI <dri-devel@lists.freedesktop.org>,
	IDE-ML <linux-ide@vger.kernel.org>,
	VMware Graphics <linux-graphics-maintainer@vmware.com>,
	Tejun Heo <tj@kernel.org>, Brian Paul <brianp@vmware.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: [PATCH, RESEND 03/14] drm/vmwgfx: avoid gcc-7 parentheses warning
Date: Mon, 17 Jul 2017 15:15:49 +0200	[thread overview]
Message-ID: <20170717131548.GA15306@pc24.home> (raw)
In-Reply-To: <CAK8P3a2QEOuQXy51q-EqzTh3STKTDHy2V-twi5nFPbuzOSEDkQ@mail.gmail.com>

On Fri, Jul 14, 2017 at 10:28:29PM +0200, Arnd Bergmann wrote:
> On Fri, Jul 14, 2017 at 9:23 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> > On Fri, Jul 14, 2017 at 12:21 PM, Linus Torvalds
> > <torvalds@linux-foundation.org> 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.

Correct.  I'll fix this.

> >
> > 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.

I've done some digging and noticed that my -fixes pull request
didn't get picked up last December.  It's most likely because I
initially made an address typo in the original request, and then
followed it up with a direct email with the correct address.

Sinclair

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel