All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tilman Schmidt <tilman@imap.cc>
To: Paul Bolle <pebolle@tiscali.nl>, netdev@vger.kernel.org
Cc: "Dmitry Vyukov" <dvyukov@google.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Martin Wilck" <Martin.Wilck@ts.fujitsu.com>,
	"Jarkko Sakkinen" <jarkko.sakkinen@linux.intel.com>,
	linux-kernel@vger.kernel.org,
	gigaset307x-common@lists.sourceforge.net
Subject: Re: [PATCH 1/1] ser_gigaset: use container_of() instead of detour
Date: Thu, 18 Feb 2016 22:42:15 +0100	[thread overview]
Message-ID: <56C63AB7.9060401@imap.cc> (raw)
In-Reply-To: <1455827348-8574-2-git-send-email-pebolle@tiscali.nl>

[-- Attachment #1: Type: text/plain, Size: 2084 bytes --]

Am 18.02.2016 um 21:29 schrieb Paul Bolle:
> The purpose of gigaset_device_release() is to kfree() the struct
> ser_cardstate that contains our struct device. This is done via a bit of
> a detour. First we make our struct device's driver_data point to the
> container of our struct ser_cardstate (which is a struct cardstate). In
> gigaset_device_release() we then retrieve that driver_data again. And
> after that we finally kfree() the struct ser_cardstate that was saved in
> the struct cardstate.
> 
> All of this can be achieved much easier by using container_of() to get
> from our struct device to its container, struct ser_cardstate. Do so.

You're absolutely right. Very nice!

> Note that at the time the detour was implemented commit b8b2c7d845d5
> ("base/platform: assert that dev_pm_domain callbacks are called
> unconditionally") had just entered the tree. That commit disconnected
> our platform_device and our platform_driver. These were reconnected
> again in v4.5-rc2 through commit 25cad69f21f5 ("base/platform: Fix
> platform drivers with no probe callback"). And one of the consequences
> of that fix was that it broke the detour via driver_data. That's because
> it made __device_release_driver() stop being a NOP for our struct device
> and actually do stuff again. One of the things it now does, is setting
> our driver_data to NULL. That, in turn, makes it impossible for
> gigaset_device_release() to get to our struct cardstate. Which has the
> net effect of leaking a struct ser_cardstate at every call of this
> driver's tty close() operation. So using container_of() has the
> additional benefit of actually working.
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Tested-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

Acked-by: Tilman Schmidt <tilman@imap.cc>

Thanks for cleaning up the mess I left behind.

Tilman

-- 
Tilman Schmidt                              E-Mail: tilman@imap.cc
Bonn, Germany
Nous, on a des fleurs et des bougies pour nous protéger.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  reply	other threads:[~2016-02-18 21:42 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-18 20:29 [PATCH 0/1] ser_gigaset: use container_of() instead of detour Paul Bolle
2016-02-18 20:29 ` [PATCH 1/1] " Paul Bolle
2016-02-18 21:42   ` Tilman Schmidt [this message]
2016-02-18 22:14     ` Paul Bolle
2016-02-18 23:46   ` Greg KH
2016-02-18 23:53     ` Paul Bolle
2016-02-19 20:56   ` David Miller

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=56C63AB7.9060401@imap.cc \
    --to=tilman@imap.cc \
    --cc=Martin.Wilck@ts.fujitsu.com \
    --cc=dvyukov@google.com \
    --cc=gigaset307x-common@lists.sourceforge.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jarkko.sakkinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pebolle@tiscali.nl \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.