mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: James Bottomley <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>
Subject: Re: scsi: ses: Move a label in ses_enclosure_data_process()
Date: Sat, 30 Dec 2023 08:45:59 -0500	[thread overview]
Message-ID: <4018ab9225ecaf18501e54114a94217a58a8a57f.camel@linux.ibm.com> (raw)
In-Reply-To: <a3825ab2-8987-4b85-9db0-642035789c49@web.de>

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


  reply	other threads:[~2023-12-30 13:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-28 14:48 [PATCH] " Markus Elfring
2023-12-29 17:21 ` James Bottomley
2023-12-29 17:29   ` Julia Lawall
2023-12-30  7:04   ` Markus Elfring
2023-12-30 12:41     ` James Bottomley
2023-12-30 13:36       ` Markus Elfring
2023-12-30 13:45         ` James Bottomley [this message]
2023-12-30 14:25           ` Markus Elfring
2023-12-31 14:07             ` James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4018ab9225ecaf18501e54114a94217a58a8a57f.camel@linux.ibm.com \
    --to=jejb@linux.ibm.com \
    --cc=Markus.Elfring@web.de \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®