Linux PCI subsystem development
 help / color / mirror / Atom feed
From: Keith Busch <kbusch@kernel.org>
To: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 2/3] PCI/sysfs: Use __free() in reset_method_store()
Date: Wed, 30 Oct 2024 17:18:15 -0600	[thread overview]
Message-ID: <ZyK-typQQqk0KKPf@kbusch-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <d07973e2-0ac1-d3e0-25f0-7b1270ca4a15@linux.intel.com>

On Mon, Oct 28, 2024 at 08:18:39PM +0200, Ilpo Järvinen wrote:
> I went to archives and found out it had already made itself into 
> include/linux/cleanup.h which now says this:
> 
> "
>  * Now, when a function uses both __free() and guard(), or multiple
>  * instances of __free(), the LIFO order of variable definition order
>  * matters. GCC documentation says:
>  *
>  * "When multiple variables in the same scope have cleanup attributes,
>  * at exit from the scope their associated cleanup functions are run in
>  * reverse order of definition (last defined, first cleanup)."
>  *
>  * When the unwind order matters it requires that variables be defined
>  * mid-function scope rather than at the top of the file.
> 
> [...snip examples...]
> 
>  * Given that the "__free(...) = NULL" pattern for variables defined at
>  * the top of the function poses this potential interdependency problem
>  * the recommendation is to always define and assign variables in one
>  * statement and not group variable definitions at the top of the
>  * function when __free() is used.
> "
> 
> After reading the documentation for real now myself :-), I realized it's
> not just about maintainer preferences but about order of releasing things, 
> so it's a BAD PATTERN to put those declarations into the usual place when
> using __free().

Okay, I can't argue against that... It still looks unpleasant. :)

  reply	other threads:[~2024-10-30 23:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-28 17:40 [PATCH 0/3] PCI/sysfs: move to sysfs funcs to pci-sysfs.c & do small tweaks Ilpo Järvinen
2024-10-28 17:40 ` [PATCH 1/3] PCI/sysfs: Move reset related sysfs code to correct file Ilpo Järvinen
2024-10-28 17:40 ` [PATCH 2/3] PCI/sysfs: Use __free() in reset_method_store() Ilpo Järvinen
2024-10-28 17:53   ` Keith Busch
2024-10-28 17:59     ` Ilpo Järvinen
2024-10-28 18:18       ` Ilpo Järvinen
2024-10-30 23:18         ` Keith Busch [this message]
2024-10-28 17:40 ` [PATCH 3/3] PCI/sysfs: Remove unnecessary zero in initializer Ilpo Järvinen
2025-01-15 11:56 ` [PATCH 0/3] PCI/sysfs: move to sysfs funcs to pci-sysfs.c & do small tweaks Krzysztof Wilczyński

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=ZyK-typQQqk0KKPf@kbusch-mbp.dhcp.thefacebook.com \
    --to=kbusch@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    /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