* [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced'
@ 2013-04-15 13:27 Daniel Borkmann
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daniel Borkmann @ 2013-04-15 13:27 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
There is actually no need to keep this member in the structure, because
after init it's always 1 anyway, thus always kfree called. This seems to
be an ancient leftover from the very initial implementation from 2.5
times. Only in case the initialization of an association fails, we leave
base.malloced as 0, but we nevertheless kfree it in the error path in
sctp_association_new().
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
Likely, there are also other such candidates in SCTP. Will have a look
for some follow-ups.
include/net/sctp/structs.h | 2 --
net/sctp/associola.c | 8 ++------
net/sctp/endpointola.c | 10 +++-------
3 files changed, 5 insertions(+), 15 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0e0f9d2..3e80eed 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1174,11 +1174,9 @@ struct sctp_ep_common {
/* Some fields to help us manage this object.
* refcnt - Reference count access to this object.
* dead - Do not attempt to use this object.
- * malloced - Do we need to kfree this object?
*/
atomic_t refcnt;
char dead;
- char malloced;
/* What socket does this endpoint belong to? */
struct sock *sk;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index d2709e2..b893aa6 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -105,7 +105,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
/* Initialize the object handling fields. */
atomic_set(&asoc->base.refcnt, 1);
asoc->base.dead = 0;
- asoc->base.malloced = 0;
/* Initialize the bind addr area. */
sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
@@ -371,7 +370,6 @@ struct sctp_association *sctp_association_new(const struct sctp_endpoint *ep,
if (!sctp_association_init(asoc, ep, sk, scope, gfp))
goto fail_init;
- asoc->base.malloced = 1;
SCTP_DBG_OBJCNT_INC(assoc);
SCTP_DEBUG_PRINTK("Created asoc %p\n", asoc);
@@ -484,10 +482,8 @@ static void sctp_association_destroy(struct sctp_association *asoc)
WARN_ON(atomic_read(&asoc->rmem_alloc));
- if (asoc->base.malloced) {
- kfree(asoc);
- SCTP_DBG_OBJCNT_DEC(assoc);
- }
+ kfree(asoc);
+ SCTP_DBG_OBJCNT_DEC(assoc);
}
/* Change the primary destination address for the peer. */
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 12ed45d..46bbfc2 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -122,7 +122,6 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
/* Initialize the basic object fields. */
atomic_set(&ep->base.refcnt, 1);
ep->base.dead = 0;
- ep->base.malloced = 1;
/* Create an input queue. */
sctp_inq_init(&ep->base.inqueue);
@@ -198,7 +197,7 @@ struct sctp_endpoint *sctp_endpoint_new(struct sock *sk, gfp_t gfp)
goto fail;
if (!sctp_endpoint_init(ep, sk, gfp))
goto fail_init;
- ep->base.malloced = 1;
+
SCTP_DBG_OBJCNT_INC(ep);
return ep;
@@ -279,11 +278,8 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
if (ep->base.sk)
sock_put(ep->base.sk);
- /* Finally, free up our memory. */
- if (ep->base.malloced) {
- kfree(ep);
- SCTP_DBG_OBJCNT_DEC(ep);
- }
+ kfree(ep);
+ SCTP_DBG_OBJCNT_DEC(ep);
}
/* Hold a reference to an endpoint. */
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool
2013-04-15 13:27 [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Daniel Borkmann
@ 2013-04-15 13:27 ` Daniel Borkmann
2013-04-15 14:05 ` Vlad Yasevich
2013-04-15 18:14 ` David Miller
2013-04-15 14:04 ` [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Vlad Yasevich
2013-04-15 18:14 ` David Miller
2 siblings, 2 replies; 6+ messages in thread
From: Daniel Borkmann @ 2013-04-15 13:27 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Since dead only holds two states (0,1), make it a bool instead
of a 'char', which is more appropriate for its purpose.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
include/net/sctp/structs.h | 2 +-
net/sctp/associola.c | 4 ++--
net/sctp/endpointola.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 3e80eed..e12aa77 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1176,7 +1176,7 @@ struct sctp_ep_common {
* dead - Do not attempt to use this object.
*/
atomic_t refcnt;
- char dead;
+ bool dead;
/* What socket does this endpoint belong to? */
struct sock *sk;
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index b893aa6..423549a 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -104,7 +104,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
/* Initialize the object handling fields. */
atomic_set(&asoc->base.refcnt, 1);
- asoc->base.dead = 0;
+ asoc->base.dead = false;
/* Initialize the bind addr area. */
sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
@@ -407,7 +407,7 @@ void sctp_association_free(struct sctp_association *asoc)
/* Mark as dead, so other users can know this structure is
* going away.
*/
- asoc->base.dead = 1;
+ asoc->base.dead = true;
/* Dispose of any data lying around in the outqueue. */
sctp_outq_free(&asoc->outqueue);
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index 46bbfc2..5fbd7bc 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -121,7 +121,7 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
/* Initialize the basic object fields. */
atomic_set(&ep->base.refcnt, 1);
- ep->base.dead = 0;
+ ep->base.dead = false;
/* Create an input queue. */
sctp_inq_init(&ep->base.inqueue);
@@ -233,7 +233,7 @@ void sctp_endpoint_add_asoc(struct sctp_endpoint *ep,
*/
void sctp_endpoint_free(struct sctp_endpoint *ep)
{
- ep->base.dead = 1;
+ ep->base.dead = true;
ep->base.sk->sk_state = SCTP_SS_CLOSED;
--
1.7.11.7
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
@ 2013-04-15 14:05 ` Vlad Yasevich
2013-04-15 18:14 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-15 14:05 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
On 04/15/2013 09:27 AM, Daniel Borkmann wrote:
> Since dead only holds two states (0,1), make it a bool instead
> of a 'char', which is more appropriate for its purpose.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
> ---
> include/net/sctp/structs.h | 2 +-
> net/sctp/associola.c | 4 ++--
> net/sctp/endpointola.c | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 3e80eed..e12aa77 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1176,7 +1176,7 @@ struct sctp_ep_common {
> * dead - Do not attempt to use this object.
> */
> atomic_t refcnt;
> - char dead;
> + bool dead;
>
> /* What socket does this endpoint belong to? */
> struct sock *sk;
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index b893aa6..423549a 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -104,7 +104,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
>
> /* Initialize the object handling fields. */
> atomic_set(&asoc->base.refcnt, 1);
> - asoc->base.dead = 0;
> + asoc->base.dead = false;
>
> /* Initialize the bind addr area. */
> sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
> @@ -407,7 +407,7 @@ void sctp_association_free(struct sctp_association *asoc)
> /* Mark as dead, so other users can know this structure is
> * going away.
> */
> - asoc->base.dead = 1;
> + asoc->base.dead = true;
>
> /* Dispose of any data lying around in the outqueue. */
> sctp_outq_free(&asoc->outqueue);
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index 46bbfc2..5fbd7bc 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -121,7 +121,7 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
>
> /* Initialize the basic object fields. */
> atomic_set(&ep->base.refcnt, 1);
> - ep->base.dead = 0;
> + ep->base.dead = false;
>
> /* Create an input queue. */
> sctp_inq_init(&ep->base.inqueue);
> @@ -233,7 +233,7 @@ void sctp_endpoint_add_asoc(struct sctp_endpoint *ep,
> */
> void sctp_endpoint_free(struct sctp_endpoint *ep)
> {
> - ep->base.dead = 1;
> + ep->base.dead = true;
>
> ep->base.sk->sk_state = SCTP_SS_CLOSED;
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
2013-04-15 14:05 ` Vlad Yasevich
@ 2013-04-15 18:14 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2013-04-15 18:14 UTC (permalink / raw)
To: dborkman; +Cc: netdev, linux-sctp
From: Daniel Borkmann <dborkman@redhat.com>
Date: Mon, 15 Apr 2013 15:27:18 +0200
> Since dead only holds two states (0,1), make it a bool instead
> of a 'char', which is more appropriate for its purpose.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced'
2013-04-15 13:27 [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Daniel Borkmann
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
@ 2013-04-15 14:04 ` Vlad Yasevich
2013-04-15 18:14 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Vlad Yasevich @ 2013-04-15 14:04 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
On 04/15/2013 09:27 AM, Daniel Borkmann wrote:
> There is actually no need to keep this member in the structure, because
> after init it's always 1 anyway, thus always kfree called. This seems to
> be an ancient leftover from the very initial implementation from 2.5
> times. Only in case the initialization of an association fails, we leave
> base.malloced as 0, but we nevertheless kfree it in the error path in
> sctp_association_new().
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
> ---
> Likely, there are also other such candidates in SCTP. Will have a look
> for some follow-ups.
>
> include/net/sctp/structs.h | 2 --
> net/sctp/associola.c | 8 ++------
> net/sctp/endpointola.c | 10 +++-------
> 3 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index 0e0f9d2..3e80eed 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1174,11 +1174,9 @@ struct sctp_ep_common {
> /* Some fields to help us manage this object.
> * refcnt - Reference count access to this object.
> * dead - Do not attempt to use this object.
> - * malloced - Do we need to kfree this object?
> */
> atomic_t refcnt;
> char dead;
> - char malloced;
>
> /* What socket does this endpoint belong to? */
> struct sock *sk;
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index d2709e2..b893aa6 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -105,7 +105,6 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
> /* Initialize the object handling fields. */
> atomic_set(&asoc->base.refcnt, 1);
> asoc->base.dead = 0;
> - asoc->base.malloced = 0;
>
> /* Initialize the bind addr area. */
> sctp_bind_addr_init(&asoc->base.bind_addr, ep->base.bind_addr.port);
> @@ -371,7 +370,6 @@ struct sctp_association *sctp_association_new(const struct sctp_endpoint *ep,
> if (!sctp_association_init(asoc, ep, sk, scope, gfp))
> goto fail_init;
>
> - asoc->base.malloced = 1;
> SCTP_DBG_OBJCNT_INC(assoc);
> SCTP_DEBUG_PRINTK("Created asoc %p\n", asoc);
>
> @@ -484,10 +482,8 @@ static void sctp_association_destroy(struct sctp_association *asoc)
>
> WARN_ON(atomic_read(&asoc->rmem_alloc));
>
> - if (asoc->base.malloced) {
> - kfree(asoc);
> - SCTP_DBG_OBJCNT_DEC(assoc);
> - }
> + kfree(asoc);
> + SCTP_DBG_OBJCNT_DEC(assoc);
> }
>
> /* Change the primary destination address for the peer. */
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index 12ed45d..46bbfc2 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -122,7 +122,6 @@ static struct sctp_endpoint *sctp_endpoint_init(struct sctp_endpoint *ep,
> /* Initialize the basic object fields. */
> atomic_set(&ep->base.refcnt, 1);
> ep->base.dead = 0;
> - ep->base.malloced = 1;
>
> /* Create an input queue. */
> sctp_inq_init(&ep->base.inqueue);
> @@ -198,7 +197,7 @@ struct sctp_endpoint *sctp_endpoint_new(struct sock *sk, gfp_t gfp)
> goto fail;
> if (!sctp_endpoint_init(ep, sk, gfp))
> goto fail_init;
> - ep->base.malloced = 1;
> +
> SCTP_DBG_OBJCNT_INC(ep);
> return ep;
>
> @@ -279,11 +278,8 @@ static void sctp_endpoint_destroy(struct sctp_endpoint *ep)
> if (ep->base.sk)
> sock_put(ep->base.sk);
>
> - /* Finally, free up our memory. */
> - if (ep->base.malloced) {
> - kfree(ep);
> - SCTP_DBG_OBJCNT_DEC(ep);
> - }
> + kfree(ep);
> + SCTP_DBG_OBJCNT_DEC(ep);
> }
>
> /* Hold a reference to an endpoint. */
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced'
2013-04-15 13:27 [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Daniel Borkmann
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
2013-04-15 14:04 ` [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Vlad Yasevich
@ 2013-04-15 18:14 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-04-15 18:14 UTC (permalink / raw)
To: dborkman; +Cc: netdev, linux-sctp
From: Daniel Borkmann <dborkman@redhat.com>
Date: Mon, 15 Apr 2013 15:27:17 +0200
> There is actually no need to keep this member in the structure, because
> after init it's always 1 anyway, thus always kfree called. This seems to
> be an ancient leftover from the very initial implementation from 2.5
> times. Only in case the initialization of an association fails, we leave
> base.malloced as 0, but we nevertheless kfree it in the error path in
> sctp_association_new().
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-04-15 18:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-15 13:27 [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Daniel Borkmann
2013-04-15 13:27 ` [PATCH net-next 2/2] net: sctp: minor: make sctp_ep_common's member 'dead' a bool Daniel Borkmann
2013-04-15 14:05 ` Vlad Yasevich
2013-04-15 18:14 ` David Miller
2013-04-15 14:04 ` [PATCH net-next 1/2] net: sctp: remove sctp_ep_common struct member 'malloced' Vlad Yasevich
2013-04-15 18:14 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).