Received-SPF: pass (google.com: domain of linux-kernel+bounces-13437-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <4018ab9225ecaf18501e54114a94217a58a8a57f.camel@linux.ibm.com>
Subject: Re: scsi: ses: Move a label in ses_enclosure_data_process()
From: James Bottomley <jejb@linux.ibm.com>
Reply-To: jejb@linux.ibm.com
To: Markus Elfring <Markus.Elfring@web.de>, linux-scsi@vger.kernel.org,
        kernel-janitors@vger.kernel.org,
        "Martin K. Petersen"
	 <martin.petersen@oracle.com>
Cc: LKML <linux-kernel@vger.kernel.org>
Date: Sat, 30 Dec 2023 08:45:59 -0500
In-Reply-To: <a3825ab2-8987-4b85-9db0-642035789c49@web.de>
References: <4616e325-e313-4078-9788-dd1e6e51b9e0@web.de>
	 <9d24844f30604f969ac10da456801f594ce72f2d.camel@linux.ibm.com>
	 <b65afa15-41e6-4d71-87bd-39fd688fa551@web.de>
	 <a35c6128d4449fec00238c909e5f6f45ebf4bcba.camel@linux.ibm.com>
	 <a3825ab2-8987-4b85-9db0-642035789c49@web.de>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0

On Sat, 2023-12-30 at 14:36 +0100, Markus Elfring wrote:
> > > You probably know some advices from another information source.
> > > 
> > > https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources
> > > 
> > 
> > Yes, but it's about using staged deallocation at the end of the
> > function instead of in the if loops.  That's to *simplify* the exit
> > chain and make the error legs less error prone because the teardown
> > isn't repeated in if bodies.  It has no bearing on what you just
> > tried to do.
> 
> I got the impression that there is a general conflict involved
> according to different programming styles.

There isn't; you simply seem to have misunderstood what the above 
resource is actually saying.

> > > >                                               If coccinelle
> > > > suddenly thinks this is a problem, it's coccinelle that needs
> > > > fixing.
> > > 
> > > This software tool can help to point source code places out for
> > > further considerations. The search patterns are evolving
> > > accordingly.
> > 
> > The pattern is wrong because kfree(NULL) exists as a teardown
> > simplification.
> 
> It might be convenient to view in this way.
> 
> If you would dare to follow advice from goto chains in a strict way,
> I imagine that you can tend to stress the attention for more useful
> data processing a bit more than such a redundant function call.

It's about maintainability and simplicity.  Eliminating kfree(NULL)
doesn't simplify most code, it just makes the exit paths more complex
as I've said for the third time now.  It could possibly be done in
extremely performance critical situations for good and well documented
reason, but that's only a tiny fraction of the kernel and it certainly
doesn't apply here.

James