From: Oliver Hartkopp <socketcan@hartkopp.net>
To: Eric Dumazet <eric.dumazet@gmail.com>,
Alexander Duyck <alexander.h.duyck@intel.com>
Cc: Florian Westphal <fw@strlen.de>,
netdev@vger.kernel.org, mkl@pengutronix.de,
"linux-can@vger.kernel.org" <linux-can@vger.kernel.org>
Subject: Re: [PATCH] can: use sock_efree instead of own destructor
Date: Tue, 10 Mar 2015 14:36:21 +0100 [thread overview]
Message-ID: <54FEF355.305@hartkopp.net> (raw)
In-Reply-To: <1425990143.8261.28.camel@edumazet-glaptop2.roam.corp.google.com>
Yes - in connection with sock_rfree() for the read buffer destructur and
sock_wfree() for the write buffer it can make sense to name a function
sock_efree() as an unassigned destructor - which does not fiddle with rmem nor
wmen.
But both sock_efree() and sock_edemux() lack some comment - especially when it
makes sense to use them from non-INET contexts which Florian suggested.
Maybe Alexander can send a patch which adds a comment, as I don't know if I
would find the best words for it.
Regards,
Oliver
On 03/10/2015 01:22 PM, Eric Dumazet wrote:
> On Tue, 2015-03-10 at 07:14 +0100, Oliver Hartkopp wrote:
>
>> the other callers use it in the same way so it's a good simplification.
>> Btw. the name of sock_efree() is a bit misleading - nothing is free'd here.
>>
>> Won't it be better to rename sock_efree(skb) with sock_put_skb(skb) or
>> something like that? sock_efree() has no comment why it's named like this.
>
> I would prefer name stays as is. It eases searches in changelogs to not
> change function names unless really needed, for backports and code
> maintenance.
>
>
> # git log | grep sock_efree
> net: merge cases where sock_efree and sock_edemux are the same function
> Since sock_efree and sock_demux are essentially the same code for non-TCP
> In addition I have added a destructor named sock_efree which is meant to
>
> sock_efree was added in commit 62bccb8cdb69051b95a55ab0c489e3cab261c8ef
>
> I guess Alexander chose the name close to sock_edemux() and sock_rfree() ones.
>
> This makes sense to me at least.
>
next prev parent reply other threads:[~2015-03-10 13:36 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-10 3:48 [PATCH] can: use sock_efree instead of own destructor Florian Westphal
2015-03-10 6:14 ` Oliver Hartkopp
2015-03-10 12:22 ` Eric Dumazet
2015-03-10 13:36 ` Oliver Hartkopp [this message]
2015-03-10 14:36 ` Eric Dumazet
2015-03-10 14:55 ` Oliver Hartkopp
2015-03-10 15:19 ` Alexander Duyck
2015-03-10 7:50 ` Marc Kleine-Budde
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=54FEF355.305@hartkopp.net \
--to=socketcan@hartkopp.net \
--cc=alexander.h.duyck@intel.com \
--cc=eric.dumazet@gmail.com \
--cc=fw@strlen.de \
--cc=linux-can@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=netdev@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 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.