Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Kousik Sanagavarapu <five231003@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>,
	kernel test robot <lkp@intel.com>, Nishanth Menon <nm@ti.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	Julia Lawall <julia.lawall@inria.fr>, <llvm@lists.linux.dev>,
	<oe-kbuild-all@lists.linux.dev>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Javier Carrasco <javier.carrasco.cruz@gmail.com>,
	<linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 2/3] soc: ti: knav_qmss_queue: do device_node auto cleanup
Date: Thu, 16 May 2024 15:48:23 +0100	[thread overview]
Message-ID: <20240516154823.00000974@Huawei.com> (raw)
In-Reply-To: <ZkG_7wJvIjZ4ZlcV@five231003>

On Mon, 13 May 2024 12:53:27 +0530
Kousik Sanagavarapu <five231003@gmail.com> wrote:

> On Sun, May 12, 2024 at 11:44:51PM -0700, Nathan Chancellor wrote:
> > On Sun, May 12, 2024 at 03:56:22PM +0530, Kousik Sanagavarapu wrote:  
> > > On Sat, May 11, 2024 at 06:12:39PM +0800, kernel test robot wrote:  
> > > > Hi Kousik,
> > > > 
> > > > kernel test robot noticed the following build errors:
> > > >   
> > > 
> > > [...]
> > >   
> > > > All errors (new ones prefixed by >>):
> > > >   
> > > > >> drivers/soc/ti/knav_qmss_queue.c:1853:3: error: cannot jump from this goto statement to its label  
> > > >                    goto err;
> > > >                    ^
> > > >    drivers/soc/ti/knav_qmss_queue.c:1855:22: note: jump bypasses initialization of variable with __attribute__((cleanup))
> > > >            struct device_node *regions __free(device_node) =
> > > >                                ^  
> 
> [...]
> 
> > > Seems like gcc didn't catch this when I compiled locally.  
> > 
> > FWIW, you may notice this as you do more conversions. The fact that GCC
> > does not warn at all is a GCC bug as far as I am aware (i.e., clang's
> > error is correct):
> > 
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91951
> > 
> > which has come up in other places:
> > 
> > https://lore.kernel.org/20240425174732.GA270911@dev-arch.thelio-3990X/  
> 
> Thank you so much for these links :)
> 
> All my internet searches ended up at stackoverflow posts which didn't
> even describe the problem correctly, which also lead me to write an
> email explaining a partly erroneous solution, which is sitting in my
> mailbox ;)
> 
> Thanks again, these will help a lot.

Independent of all this, it's not a good idea form a readability point
of view to mix automated and manual cleanup.  So in cases like this
where you want to do scope based cleanup, use separate functions
that have appropriately defined scope (or brackets for the really minor
cases).

Here, you may just be able to push the device_node get into
knav_queue_setup_regions() for example.

Jonathan

> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel