Re: if() before m_free(9)

[Mark Kettenis]

> Date: Mon, 9 Jan 2017 20:12:32 +0100
> From: Martin Pieuchot 
> 
> m_free(9) handles NULL, no need to check for it beforehand.
> 
> Seems that we missed these because of the (void) cast!
> 
> ok?

ok kettenis@

> Index: netinet/ip_output.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.333
> diff -u -p -r1.333 ip_output.c
> --- netinet/ip_output.c   19 Dec 2016 09:22:24 -  1.333
> +++ netinet/ip_output.c   9 Jan 2017 17:51:52 -
> @@ -1074,8 +1074,7 @@ ip_ctloutput(int op, struct socket *so, 
>   error = ENOPROTOOPT;
>   break;
>   }
> - if (m)
> - (void)m_free(m);
> + m_free(m);
>   break;
>  
>   case PRCO_GETOPT:
> @@ -1242,15 +1241,13 @@ ip_pcbopts(struct mbuf **pcbopt, struct 
>   u_char opt;
>  
>   /* turn off any old options */
> - if (*pcbopt)
> - (void)m_free(*pcbopt);
> + m_free(*pcbopt);
>   *pcbopt = 0;
>   if (m == NULL || m->m_len == 0) {
>   /*
>* Only turning off any previous options.
>*/
> - if (m)
> - (void)m_free(m);
> + m_free(m);
>   return (0);
>   }
>  
> Index: netinet/raw_ip.c
> ===
> RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 raw_ip.c
> --- netinet/raw_ip.c  19 Dec 2016 09:22:24 -  1.90
> +++ netinet/raw_ip.c  9 Jan 2017 17:52:13 -
> @@ -325,8 +325,7 @@ rip_ctloutput(int op, struct socket *so,
>   inp->inp_flags |= INP_HDRINCL;
>   else
>   inp->inp_flags &= ~INP_HDRINCL;
> - if (*mp)
> - (void)m_free(*mp);
> + m_free(*mp);
>   } else {
>   *mp = m_get(M_WAIT, M_SOOPTS);
>   (*mp)->m_len = sizeof(int);
> Index: netinet/tcp_input.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.334
> diff -u -p -r1.334 tcp_input.c
> --- netinet/tcp_input.c   19 Dec 2016 08:36:49 -  1.334
> +++ netinet/tcp_input.c   9 Jan 2017 17:53:03 -
> @@ -3340,8 +3340,7 @@ syn_cache_rm(struct syn_cache *sc)
>  void
>  syn_cache_put(struct syn_cache *sc)
>  {
> - if (sc->sc_ipopts)
> - (void) m_free(sc->sc_ipopts);
> + m_free(sc->sc_ipopts);
>   if (sc->sc_route4.ro_rt != NULL) {
>   rtfree(sc->sc_route4.ro_rt);
>   sc->sc_route4.ro_rt = NULL;
> @@ -4035,8 +4034,7 @@ syn_cache_add(struct sockaddr *src, stru
>* If we were remembering a previous source route,
>* forget it and use the new one we've been given.
>*/
> - if (sc->sc_ipopts)
> - (void) m_free(sc->sc_ipopts);
> + m_free(sc->sc_ipopts);
>   sc->sc_ipopts = ipopts;
>   }
>   sc->sc_timestamp = tb.ts_recent;
> @@ -4049,8 +4047,7 @@ syn_cache_add(struct sockaddr *src, stru
>  
>   sc = pool_get(_cache_pool, PR_NOWAIT|PR_ZERO);
>   if (sc == NULL) {
> - if (ipopts)
> - (void) m_free(ipopts);
> + m_free(ipopts);
>   return (-1);
>   }
>  
> Index: netinet/tcp_subr.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
> retrieving revision 1.157
> diff -u -p -r1.157 tcp_subr.c
> --- netinet/tcp_subr.c20 Dec 2016 09:57:10 -  1.157
> +++ netinet/tcp_subr.c9 Jan 2017 17:52:39 -
> @@ -534,8 +534,7 @@ tcp_close(struct tcpcb *tp)
>   p = q;
>   }
>  #endif
> - if (tp->t_template)
> - (void) m_free(tp->t_template);
> + m_free(tp->t_template);
>  
>   tp->t_flags |= TF_DEAD;
>   timeout_add(>t_reap_to, 0);
> Index: netinet/tcp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 tcp_usrreq.c
> --- netinet/tcp_usrreq.c  3 Jan 2017 10:52:21 -   1.141
> +++ netinet/tcp_usrreq.c  9 Jan 2017 17:52:28 -
> @@ -569,8 +569,7 @@ tcp_ctloutput(int op, struct socket *so,
>   error = ENOPROTOOPT;
>   break;
>   }
> - if (m)
> - (void) m_free(m);
> + m_free(m);
>   break;
>  
>   case PRCO_GETOPT:
> Index: netinet6/ip6_output.c
> ===
> RCS 

Re: if() before m_free(9)

[Alexander Bluhm]

On Mon, Jan 09, 2017 at 08:12:32PM +0100, Martin Pieuchot wrote:
> m_free(9) handles NULL, no need to check for it beforehand.
> 
> Seems that we missed these because of the (void) cast!
> 
> ok?

OK bluhm@

> 
> Index: netinet/ip_output.c
> ===
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.333
> diff -u -p -r1.333 ip_output.c
> --- netinet/ip_output.c   19 Dec 2016 09:22:24 -  1.333
> +++ netinet/ip_output.c   9 Jan 2017 17:51:52 -
> @@ -1074,8 +1074,7 @@ ip_ctloutput(int op, struct socket *so, 
>   error = ENOPROTOOPT;
>   break;
>   }
> - if (m)
> - (void)m_free(m);
> + m_free(m);
>   break;
>  
>   case PRCO_GETOPT:
> @@ -1242,15 +1241,13 @@ ip_pcbopts(struct mbuf **pcbopt, struct 
>   u_char opt;
>  
>   /* turn off any old options */
> - if (*pcbopt)
> - (void)m_free(*pcbopt);
> + m_free(*pcbopt);
>   *pcbopt = 0;
>   if (m == NULL || m->m_len == 0) {
>   /*
>* Only turning off any previous options.
>*/
> - if (m)
> - (void)m_free(m);
> + m_free(m);
>   return (0);
>   }
>  
> Index: netinet/raw_ip.c
> ===
> RCS file: /cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.90
> diff -u -p -r1.90 raw_ip.c
> --- netinet/raw_ip.c  19 Dec 2016 09:22:24 -  1.90
> +++ netinet/raw_ip.c  9 Jan 2017 17:52:13 -
> @@ -325,8 +325,7 @@ rip_ctloutput(int op, struct socket *so,
>   inp->inp_flags |= INP_HDRINCL;
>   else
>   inp->inp_flags &= ~INP_HDRINCL;
> - if (*mp)
> - (void)m_free(*mp);
> + m_free(*mp);
>   } else {
>   *mp = m_get(M_WAIT, M_SOOPTS);
>   (*mp)->m_len = sizeof(int);
> Index: netinet/tcp_input.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_input.c,v
> retrieving revision 1.334
> diff -u -p -r1.334 tcp_input.c
> --- netinet/tcp_input.c   19 Dec 2016 08:36:49 -  1.334
> +++ netinet/tcp_input.c   9 Jan 2017 17:53:03 -
> @@ -3340,8 +3340,7 @@ syn_cache_rm(struct syn_cache *sc)
>  void
>  syn_cache_put(struct syn_cache *sc)
>  {
> - if (sc->sc_ipopts)
> - (void) m_free(sc->sc_ipopts);
> + m_free(sc->sc_ipopts);
>   if (sc->sc_route4.ro_rt != NULL) {
>   rtfree(sc->sc_route4.ro_rt);
>   sc->sc_route4.ro_rt = NULL;
> @@ -4035,8 +4034,7 @@ syn_cache_add(struct sockaddr *src, stru
>* If we were remembering a previous source route,
>* forget it and use the new one we've been given.
>*/
> - if (sc->sc_ipopts)
> - (void) m_free(sc->sc_ipopts);
> + m_free(sc->sc_ipopts);
>   sc->sc_ipopts = ipopts;
>   }
>   sc->sc_timestamp = tb.ts_recent;
> @@ -4049,8 +4047,7 @@ syn_cache_add(struct sockaddr *src, stru
>  
>   sc = pool_get(_cache_pool, PR_NOWAIT|PR_ZERO);
>   if (sc == NULL) {
> - if (ipopts)
> - (void) m_free(ipopts);
> + m_free(ipopts);
>   return (-1);
>   }
>  
> Index: netinet/tcp_subr.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
> retrieving revision 1.157
> diff -u -p -r1.157 tcp_subr.c
> --- netinet/tcp_subr.c20 Dec 2016 09:57:10 -  1.157
> +++ netinet/tcp_subr.c9 Jan 2017 17:52:39 -
> @@ -534,8 +534,7 @@ tcp_close(struct tcpcb *tp)
>   p = q;
>   }
>  #endif
> - if (tp->t_template)
> - (void) m_free(tp->t_template);
> + m_free(tp->t_template);
>  
>   tp->t_flags |= TF_DEAD;
>   timeout_add(>t_reap_to, 0);
> Index: netinet/tcp_usrreq.c
> ===
> RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
> retrieving revision 1.141
> diff -u -p -r1.141 tcp_usrreq.c
> --- netinet/tcp_usrreq.c  3 Jan 2017 10:52:21 -   1.141
> +++ netinet/tcp_usrreq.c  9 Jan 2017 17:52:28 -
> @@ -569,8 +569,7 @@ tcp_ctloutput(int op, struct socket *so,
>   error = ENOPROTOOPT;
>   break;
>   }
> - if (m)
> - (void) m_free(m);
> + m_free(m);
>   break;
>  
>   case PRCO_GETOPT:
> Index: netinet6/ip6_output.c
> ===
> RCS file: 

if() before m_free(9)

[Martin Pieuchot]

m_free(9) handles NULL, no need to check for it beforehand.

Seems that we missed these because of the (void) cast!

ok?

Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.333
diff -u -p -r1.333 ip_output.c
--- netinet/ip_output.c 19 Dec 2016 09:22:24 -  1.333
+++ netinet/ip_output.c 9 Jan 2017 17:51:52 -
@@ -1074,8 +1074,7 @@ ip_ctloutput(int op, struct socket *so, 
error = ENOPROTOOPT;
break;
}
-   if (m)
-   (void)m_free(m);
+   m_free(m);
break;
 
case PRCO_GETOPT:
@@ -1242,15 +1241,13 @@ ip_pcbopts(struct mbuf **pcbopt, struct 
u_char opt;
 
/* turn off any old options */
-   if (*pcbopt)
-   (void)m_free(*pcbopt);
+   m_free(*pcbopt);
*pcbopt = 0;
if (m == NULL || m->m_len == 0) {
/*
 * Only turning off any previous options.
 */
-   if (m)
-   (void)m_free(m);
+   m_free(m);
return (0);
}
 
Index: netinet/raw_ip.c
===
RCS file: /cvs/src/sys/netinet/raw_ip.c,v
retrieving revision 1.90
diff -u -p -r1.90 raw_ip.c
--- netinet/raw_ip.c19 Dec 2016 09:22:24 -  1.90
+++ netinet/raw_ip.c9 Jan 2017 17:52:13 -
@@ -325,8 +325,7 @@ rip_ctloutput(int op, struct socket *so,
inp->inp_flags |= INP_HDRINCL;
else
inp->inp_flags &= ~INP_HDRINCL;
-   if (*mp)
-   (void)m_free(*mp);
+   m_free(*mp);
} else {
*mp = m_get(M_WAIT, M_SOOPTS);
(*mp)->m_len = sizeof(int);
Index: netinet/tcp_input.c
===
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.334
diff -u -p -r1.334 tcp_input.c
--- netinet/tcp_input.c 19 Dec 2016 08:36:49 -  1.334
+++ netinet/tcp_input.c 9 Jan 2017 17:53:03 -
@@ -3340,8 +3340,7 @@ syn_cache_rm(struct syn_cache *sc)
 void
 syn_cache_put(struct syn_cache *sc)
 {
-   if (sc->sc_ipopts)
-   (void) m_free(sc->sc_ipopts);
+   m_free(sc->sc_ipopts);
if (sc->sc_route4.ro_rt != NULL) {
rtfree(sc->sc_route4.ro_rt);
sc->sc_route4.ro_rt = NULL;
@@ -4035,8 +4034,7 @@ syn_cache_add(struct sockaddr *src, stru
 * If we were remembering a previous source route,
 * forget it and use the new one we've been given.
 */
-   if (sc->sc_ipopts)
-   (void) m_free(sc->sc_ipopts);
+   m_free(sc->sc_ipopts);
sc->sc_ipopts = ipopts;
}
sc->sc_timestamp = tb.ts_recent;
@@ -4049,8 +4047,7 @@ syn_cache_add(struct sockaddr *src, stru
 
sc = pool_get(_cache_pool, PR_NOWAIT|PR_ZERO);
if (sc == NULL) {
-   if (ipopts)
-   (void) m_free(ipopts);
+   m_free(ipopts);
return (-1);
}
 
Index: netinet/tcp_subr.c
===
RCS file: /cvs/src/sys/netinet/tcp_subr.c,v
retrieving revision 1.157
diff -u -p -r1.157 tcp_subr.c
--- netinet/tcp_subr.c  20 Dec 2016 09:57:10 -  1.157
+++ netinet/tcp_subr.c  9 Jan 2017 17:52:39 -
@@ -534,8 +534,7 @@ tcp_close(struct tcpcb *tp)
p = q;
}
 #endif
-   if (tp->t_template)
-   (void) m_free(tp->t_template);
+   m_free(tp->t_template);
 
tp->t_flags |= TF_DEAD;
timeout_add(>t_reap_to, 0);
Index: netinet/tcp_usrreq.c
===
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.141
diff -u -p -r1.141 tcp_usrreq.c
--- netinet/tcp_usrreq.c3 Jan 2017 10:52:21 -   1.141
+++ netinet/tcp_usrreq.c9 Jan 2017 17:52:28 -
@@ -569,8 +569,7 @@ tcp_ctloutput(int op, struct socket *so,
error = ENOPROTOOPT;
break;
}
-   if (m)
-   (void) m_free(m);
+   m_free(m);
break;
 
case PRCO_GETOPT:
Index: netinet6/ip6_output.c
===
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.218
diff -u -p -r1.218 ip6_output.c
--- netinet6/ip6_output.c   18 Nov 2016 02:53:47 -  1.218
+++ netinet6/ip6_output.c   9 Jan 2017 17:53:14 -
@@ -1414,8 +1414,7 @@