Re: [PATCH] drm/i915/guc: Remove bogus null check

From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Dan Carpenter <dan.carpenter@linaro.org>
Cc: Andi Shyti <andi.shyti@linux.intel.com>,
	<intel-gfx@lists.freedesktop.org>,
	kernel test robot <lkp@intel.com>,
	"Vinay Belgaumkar" <vinay.belgaumkar@intel.com>,
	John Harrison <John.C.Harrison@intel.com>
Subject: Re: [PATCH] drm/i915/guc: Remove bogus null check
Date: Wed, 3 Apr 2024 14:46:13 -0400	[thread overview]
Message-ID: <Zg2j9S81HJrHWcMd@intel.com> (raw)
In-Reply-To: <daa13e1c-d75d-4fbb-b400-4d6a633103bd@moroto.mountain>

On Fri, Mar 29, 2024 at 07:38:11PM +0300, Dan Carpenter wrote:
> On Fri, Mar 29, 2024 at 08:09:38AM +0100, Andi Shyti wrote:
> > Hi Rodrigo,
> > 
> > On Thu, Mar 28, 2024 at 09:39:17PM -0400, Rodrigo Vivi wrote:
> > > On Thu, Mar 28, 2024 at 10:41:55PM +0100, Andi Shyti wrote:
> > > > On Thu, Mar 28, 2024 at 05:31:07PM -0400, Rodrigo Vivi wrote:
> > > > > This null check is bogus because we are already using 'ce' stuff
> > > > > in many places before this function is called.
> > > > > 
> > > > > Having this here is useless and confuses static analyzer tools
> > > > > that can see:
> > > > > 
> > > > > struct intel_engine_cs *engine = ce->engine;
> > > > > 
> > > > > before this check, in the same function.
> > > > > 
> > > > > Fixes: cec82816d0d0 ("drm/i915/guc: Use context hints for GT frequency")
> > > > 
> > > > there is no need to have the Fixes tag here.
> > > 
> > > why not? I imagine distros that have this commit cec82816d0d0 and use
> > > static analyzers would also want this patch ported to silent those, no?!
> > 
> > Still... it's not a bug. The tag "Fixes:" should be used when a
> > bug is fixed, but not for harmless static analyzer reports.
> > 
> > Besides, if we want to keep the Fixes tag we should also Cc
> > stable, i guess checkpatch.pl complains about it.
> > 
> > (BTW, Cc'ed in this mail we have the inventor of the tag and he
> > can confirm after having had his morning coffee :-) ).
> > 
> 
> Good.  I keep reminding people that I invented the Fixes tag because it
> is my proudest achievement.  :)
> 
> No.  Only use Fixes tags for bug fixes.

Thanks for the clarifications and reviews. I have removed the 'Fixes:' tag
and pushed the patch as is.

> 
> regards,
> dan carpenter
> 