Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 14 Mar 2017 20:57:54 +0000 (UTC)
From:      Eric van Gyzen <vangyzen@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r315286 - in head/sys: dev/cxgb/ulp/iw_cxgb netinet
Message-ID:  <201703142057.v2EKvssk043033@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: vangyzen
Date: Tue Mar 14 20:57:54 2017
New Revision: 315286
URL: https://svnweb.freebsd.org/changeset/base/315286

Log:
  Add some ntohl() love to r315277
  
  inet_ntoa() and inet_ntoa_r() take the address in network
  byte-order.  When I removed those calls, I should have
  replaced them with ntohl() to make the hex addresses slightly
  less unreadable.  Here they are.
  
  See r315277 regarding classic blunders.
  
  vangyzen: you're deep in "no good deed" territory, it seems
      --badger
  
  Reported by:	ian
  MFC after:	3 days
  MFC when:	I finally get it right
  Sponsored by:	Dell EMC

Modified:
  head/sys/dev/cxgb/ulp/iw_cxgb/iw_cxgb_cm.c
  head/sys/netinet/igmp.c
  head/sys/netinet/in_mcast.c
  head/sys/netinet/ip_mroute.c

Modified: head/sys/dev/cxgb/ulp/iw_cxgb/iw_cxgb_cm.c
==============================================================================
--- head/sys/dev/cxgb/ulp/iw_cxgb/iw_cxgb_cm.c	Tue Mar 14 20:43:04 2017	(r315285)
+++ head/sys/dev/cxgb/ulp/iw_cxgb/iw_cxgb_cm.c	Tue Mar 14 20:57:54 2017	(r315286)
@@ -1479,7 +1479,8 @@ process_data(struct iwch_ep *ep)
 		in_getsockaddr(ep->com.so, (struct sockaddr **)&local);
 		in_getpeeraddr(ep->com.so, (struct sockaddr **)&remote);
 		CTR3(KTR_IW_CXGB, "%s local 0x%08x remote 0x%08x", __FUNCTION__,
-			local->sin_addr.s_addr, remote->sin_addr.s_addr);
+			ntohl(local->sin_addr.s_addr),
+			ntohl(remote->sin_addr.s_addr));
 		ep->com.local_addr = *local;
 		ep->com.remote_addr = *remote;
 		free(local, M_SONAME);
@@ -1538,7 +1539,7 @@ process_newconn(struct iw_cm_id *parent_
 	in_getpeeraddr(child_so, (struct sockaddr **)&remote);
 
 	CTR3(KTR_IW_CXGB, "%s remote addr 0x%08x port %d", __FUNCTION__, 
-		remote->sin_addr.s_addr, ntohs(remote->sin_port));
+		ntohl(remote->sin_addr.s_addr), ntohs(remote->sin_port));
 	child_ep->com.tdev = parent_ep->com.tdev;
 	child_ep->com.local_addr.sin_family = parent_ep->com.local_addr.sin_family;
 	child_ep->com.local_addr.sin_port = parent_ep->com.local_addr.sin_port;

Modified: head/sys/netinet/igmp.c
==============================================================================
--- head/sys/netinet/igmp.c	Tue Mar 14 20:43:04 2017	(r315285)
+++ head/sys/netinet/igmp.c	Tue Mar 14 20:57:54 2017	(r315286)
@@ -863,7 +863,7 @@ igmp_input_v2_query(struct ifnet *ifp, c
 		if (inm != NULL) {
 			CTR3(KTR_IGMPV3,
 			    "process v2 query 0x%08x on ifp %p(%s)",
-			    igmp->igmp_group.s_addr, ifp, ifp->if_xname);
+			    ntohl(igmp->igmp_group.s_addr), ifp, ifp->if_xname);
 			igmp_v2_update_group(inm, timer);
 		}
 	}
@@ -895,7 +895,7 @@ igmp_v2_update_group(struct in_multi *in
 {
 
 	CTR4(KTR_IGMPV3, "0x%08x: %s/%s timer=%d", __func__,
-	    inm->inm_addr.s_addr, inm->inm_ifp->if_xname, timer);
+	    ntohl(inm->inm_addr.s_addr), inm->inm_ifp->if_xname, timer);
 
 	IN_MULTI_LOCK_ASSERT();
 
@@ -1076,7 +1076,7 @@ igmp_input_v3_query(struct ifnet *ifp, c
 			}
 		}
 		CTR3(KTR_IGMPV3, "process v3 0x%08x query on ifp %p(%s)",
-		     igmpv3->igmp_group.s_addr, ifp, ifp->if_xname);
+		     ntohl(igmpv3->igmp_group.s_addr), ifp, ifp->if_xname);
 		/*
 		 * If there is a pending General Query response
 		 * scheduled sooner than the selected delay, no
@@ -1237,7 +1237,7 @@ igmp_input_v1_report(struct ifnet *ifp, 
 	}
 
 	CTR3(KTR_IGMPV3, "process v1 report 0x%08x on ifp %p(%s)",
-	     igmp->igmp_group.s_addr, ifp, ifp->if_xname);
+	     ntohl(igmp->igmp_group.s_addr), ifp, ifp->if_xname);
 
 	/*
 	 * IGMPv1 report suppression.
@@ -1280,7 +1280,7 @@ igmp_input_v1_report(struct ifnet *ifp, 
 		case IGMP_AWAKENING_MEMBER:
 			CTR3(KTR_IGMPV3,
 			    "report suppressed for 0x%08x on ifp %p(%s)",
-			    igmp->igmp_group.s_addr, ifp,
+			    ntohl(igmp->igmp_group.s_addr), ifp,
 			    ifp->if_xname);
 		case IGMP_SLEEPING_MEMBER:
 			inm->inm_state = IGMP_SLEEPING_MEMBER;
@@ -1288,7 +1288,7 @@ igmp_input_v1_report(struct ifnet *ifp, 
 		case IGMP_REPORTING_MEMBER:
 			CTR3(KTR_IGMPV3,
 			    "report suppressed for 0x%08x on ifp %p(%s)",
-			    igmp->igmp_group.s_addr, ifp,
+			    ntohl(igmp->igmp_group.s_addr), ifp,
 			    ifp->if_xname);
 			if (igi->igi_version == IGMP_VERSION_1)
 				inm->inm_state = IGMP_LAZY_MEMBER;
@@ -1363,7 +1363,7 @@ igmp_input_v2_report(struct ifnet *ifp, 
 		ifa_free(&ia->ia_ifa);
 
 	CTR3(KTR_IGMPV3, "process v2 report 0x%08x on ifp %p(%s)",
-	     igmp->igmp_group.s_addr, ifp, ifp->if_xname);
+	     ntohl(igmp->igmp_group.s_addr), ifp, ifp->if_xname);
 
 	/*
 	 * IGMPv2 report suppression.
@@ -1404,7 +1404,7 @@ igmp_input_v2_report(struct ifnet *ifp, 
 		case IGMP_AWAKENING_MEMBER:
 			CTR3(KTR_IGMPV3,
 			    "report suppressed for 0x%08x on ifp %p(%s)",
-			    igmp->igmp_group.s_addr, ifp, ifp->if_xname);
+			    ntohl(igmp->igmp_group.s_addr), ifp, ifp->if_xname);
 		case IGMP_LAZY_MEMBER:
 			inm->inm_state = IGMP_LAZY_MEMBER;
 			break;
@@ -1892,7 +1892,8 @@ igmp_v3_process_group_timers(struct igmp
 
 			inm_commit(inm);
 			CTR3(KTR_IGMPV3, "%s: T1 -> T0 for 0x%08x/%s", __func__,
-			    inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+			    ntohl(inm->inm_addr.s_addr),
+			    inm->inm_ifp->if_xname);
 
 			/*
 			 * If we are leaving the group for good, make sure
@@ -2340,7 +2341,7 @@ igmp_initial_join(struct in_multi *inm, 
 	int			 error, retval, syncstates;
  
 	CTR4(KTR_IGMPV3, "%s: initial join 0x%08x on ifp %p(%s)", __func__,
-	    inm->inm_addr.s_addr, inm->inm_ifp, inm->inm_ifp->if_xname);
+	    ntohl(inm->inm_addr.s_addr), inm->inm_ifp, inm->inm_ifp->if_xname);
 
 	error = 0;
 	syncstates = 1;
@@ -2450,7 +2451,7 @@ igmp_initial_join(struct in_multi *inm, 
 	if (syncstates) {
 		inm_commit(inm);
 		CTR3(KTR_IGMPV3, "%s: T1 -> T0 for 0x%08x/%s", __func__,
-		    inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+		    ntohl(inm->inm_addr.s_addr), inm->inm_ifp->if_xname);
 	}
 
 	return (error);
@@ -2466,7 +2467,7 @@ igmp_handle_state_change(struct in_multi
 	int			 retval;
 
 	CTR4(KTR_IGMPV3, "%s: state change for 0x%08x on ifp %p(%s)", __func__,
-	    inm->inm_addr.s_addr, inm->inm_ifp, inm->inm_ifp->if_xname);
+	    ntohl(inm->inm_addr.s_addr), inm->inm_ifp, inm->inm_ifp->if_xname);
 
 	ifp = inm->inm_ifp;
 
@@ -2486,7 +2487,7 @@ igmp_handle_state_change(struct in_multi
 		CTR1(KTR_IGMPV3, "%s: nothing to do", __func__);
 		inm_commit(inm);
 		CTR3(KTR_IGMPV3, "%s: T1 -> T0 for 0x%08x/%s", __func__,
-		    inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+		    ntohl(inm->inm_addr.s_addr), inm->inm_ifp->if_xname);
 		return (0);
 	}
 
@@ -2525,7 +2526,7 @@ igmp_final_leave(struct in_multi *inm, s
 	syncstates = 1;
 
 	CTR4(KTR_IGMPV3, "%s: final leave 0x%08x on ifp %p(%s)",
-	    __func__, inm->inm_addr.s_addr, inm->inm_ifp,
+	    __func__, ntohl(inm->inm_addr.s_addr), inm->inm_ifp,
 	    inm->inm_ifp->if_xname);
 
 	IN_MULTI_LOCK_ASSERT();
@@ -2568,7 +2569,7 @@ igmp_final_leave(struct in_multi *inm, s
 			}
 			CTR4(KTR_IGMPV3, "%s: Leaving 0x%08x/%s with %d "
 			    "pending retransmissions.", __func__,
-			    inm->inm_addr.s_addr,
+			    ntohl(inm->inm_addr.s_addr),
 			    inm->inm_ifp->if_xname, inm->inm_scrv);
 			if (inm->inm_scrv == 0) {
 				inm->inm_state = IGMP_NOT_MEMBER;
@@ -2602,10 +2603,11 @@ igmp_final_leave(struct in_multi *inm, s
 	if (syncstates) {
 		inm_commit(inm);
 		CTR3(KTR_IGMPV3, "%s: T1 -> T0 for 0x%08x/%s", __func__,
-		    inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+		    ntohl(inm->inm_addr.s_addr), inm->inm_ifp->if_xname);
 		inm->inm_st[1].iss_fmode = MCAST_UNDEFINED;
 		CTR3(KTR_IGMPV3, "%s: T1 now MCAST_UNDEFINED for 0x%08x/%s",
-		    __func__, inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+		    __func__, ntohl(inm->inm_addr.s_addr),
+		    inm->inm_ifp->if_xname);
 	}
 }
 
@@ -2731,7 +2733,7 @@ igmp_v3_enqueue_group_record(struct mbuf
 
 	if (type == IGMP_DO_NOTHING) {
 		CTR3(KTR_IGMPV3, "%s: nothing to do for 0x%08x/%s", __func__,
-		    inm->inm_addr.s_addr, inm->inm_ifp->if_xname);
+		    ntohl(inm->inm_addr.s_addr), inm->inm_ifp->if_xname);
 		return (0);
 	}
 
@@ -2745,7 +2747,7 @@ igmp_v3_enqueue_group_record(struct mbuf
 		minrec0len += sizeof(in_addr_t);
 
 	CTR4(KTR_IGMPV3, "%s: queueing %s for 0x%08x/%s", __func__,
-	    igmp_rec_type_to_str(type), inm->inm_addr.s_addr,
+	    igmp_rec_type_to_str(type), ntohl(inm->inm_addr.s_addr),
 	    inm->inm_ifp->if_xname);
 
 	/*
@@ -2834,7 +2836,7 @@ igmp_v3_enqueue_group_record(struct mbuf
 		msrcs = 0;
 		RB_FOREACH_SAFE(ims, ip_msource_tree, &inm->inm_srcs, nims) {
 			CTR2(KTR_IGMPV3, "%s: visit node 0x%08x", __func__,
-			    htonl(ims->ims_haddr));
+			    ims->ims_haddr);
 			now = ims_get_mode(inm, ims, 1);
 			CTR2(KTR_IGMPV3, "%s: node is %d", __func__, now);
 			if ((now != mode) ||
@@ -2930,7 +2932,7 @@ igmp_v3_enqueue_group_record(struct mbuf
 		msrcs = 0;
 		RB_FOREACH_FROM(ims, ip_msource_tree, nims) {
 			CTR2(KTR_IGMPV3, "%s: visit node 0x%08x", __func__,
-			    htonl(ims->ims_haddr));
+			    ims->ims_haddr);
 			now = ims_get_mode(inm, ims, 1);
 			if ((now != mode) ||
 			    (now == mode && mode == MCAST_UNDEFINED)) {
@@ -3122,7 +3124,7 @@ igmp_v3_enqueue_filter_change(struct mbu
 				nims = RB_MIN(ip_msource_tree, &inm->inm_srcs);
 			RB_FOREACH_FROM(ims, ip_msource_tree, nims) {
 				CTR2(KTR_IGMPV3, "%s: visit node 0x%08x",
-				    __func__, htonl(ims->ims_haddr));
+				    __func__, ims->ims_haddr);
 				now = ims_get_mode(inm, ims, 1);
 				then = ims_get_mode(inm, ims, 0);
 				CTR3(KTR_IGMPV3, "%s: mode: t0 %d, t1 %d",

Modified: head/sys/netinet/in_mcast.c
==============================================================================
--- head/sys/netinet/in_mcast.c	Tue Mar 14 20:43:04 2017	(r315285)
+++ head/sys/netinet/in_mcast.c	Tue Mar 14 20:57:54 2017	(r315286)
@@ -893,7 +893,7 @@ inm_get_source(struct in_multi *inm, con
 		ims = nims;
 #ifdef KTR
 		CTR3(KTR_IGMPV3, "%s: allocated 0x%08x as %p", __func__,
-		    htonl(haddr), ims);
+		    haddr, ims);
 #endif
 	}
 
@@ -910,29 +910,24 @@ ims_merge(struct ip_msource *ims, const 
     const int rollback)
 {
 	int n = rollback ? -1 : 1;
-#ifdef KTR
-	uint32_t addr;
-
-	addr = htonl(ims->ims_haddr);
-#endif
 
 	if (lims->imsl_st[0] == MCAST_EXCLUDE) {
 		CTR3(KTR_IGMPV3, "%s: t1 ex -= %d on 0x%08x",
-		    __func__, n, addr);
+		    __func__, n, ims->ims_haddr);
 		ims->ims_st[1].ex -= n;
 	} else if (lims->imsl_st[0] == MCAST_INCLUDE) {
 		CTR3(KTR_IGMPV3, "%s: t1 in -= %d on 0x%08x",
-		    __func__, n, addr);
+		    __func__, n, ims->ims_haddr);
 		ims->ims_st[1].in -= n;
 	}
 
 	if (lims->imsl_st[1] == MCAST_EXCLUDE) {
 		CTR3(KTR_IGMPV3, "%s: t1 ex += %d on 0x%08x",
-		    __func__, n, addr);
+		    __func__, n, ims->ims_haddr);
 		ims->ims_st[1].ex += n;
 	} else if (lims->imsl_st[1] == MCAST_INCLUDE) {
 		CTR3(KTR_IGMPV3, "%s: t1 in += %d on 0x%08x",
-		    __func__, n, addr);
+		    __func__, n, ims->ims_haddr);
 		ims->ims_st[1].in += n;
 	}
 }
@@ -1169,7 +1164,7 @@ in_joingroup_locked(struct ifnet *ifp, c
 	IN_MULTI_LOCK_ASSERT();
 
 	CTR4(KTR_IGMPV3, "%s: join 0x%08x on %p(%s))", __func__,
-	    gina->s_addr, ifp, ifp->if_xname);
+	    ntohl(gina->s_addr), ifp, ifp->if_xname);
 
 	error = 0;
 	inm = NULL;
@@ -1253,7 +1248,7 @@ in_leavegroup_locked(struct in_multi *in
 	IN_MULTI_LOCK_ASSERT();
 
 	CTR5(KTR_IGMPV3, "%s: leave inm %p, 0x%08x/%s, imf %p", __func__,
-	    inm, inm->inm_addr.s_addr,
+	    inm, ntohl(inm->inm_addr.s_addr),
 	    (inm_is_ifp_detached(inm) ? "null" : inm->inm_ifp->if_xname),
 	    imf);
 
@@ -1387,7 +1382,7 @@ inp_block_unblock_source(struct inpcb *i
 			doblock = 1;
 
 		CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p",
-		    __func__, mreqs.imr_interface.s_addr, ifp);
+		    __func__, ntohl(mreqs.imr_interface.s_addr), ifp);
 		break;
 	    }
 
@@ -1460,7 +1455,7 @@ inp_block_unblock_source(struct inpcb *i
 	ims = imo_match_source(imo, idx, &ssa->sa);
 	if ((ims != NULL && doblock) || (ims == NULL && !doblock)) {
 		CTR3(KTR_IGMPV3, "%s: source 0x%08x %spresent", __func__,
-		    ssa->sin.sin_addr.s_addr, doblock ? "" : "not ");
+		    ntohl(ssa->sin.sin_addr.s_addr), doblock ? "" : "not ");
 		error = EADDRNOTAVAIL;
 		goto out_inp_locked;
 	}
@@ -1989,7 +1984,7 @@ inp_join_group(struct inpcb *inp, struct
 		ifp = inp_lookup_mcast_ifp(inp, &gsa->sin,
 		    mreqs.imr_interface);
 		CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p",
-		    __func__, mreqs.imr_interface.s_addr, ifp);
+		    __func__, ntohl(mreqs.imr_interface.s_addr), ifp);
 		break;
 	}
 
@@ -2290,7 +2285,7 @@ inp_leave_group(struct inpcb *inp, struc
 			INADDR_TO_IFP(mreqs.imr_interface, ifp);
 
 		CTR3(KTR_IGMPV3, "%s: imr_interface = 0x%08x, ifp = %p",
-		    __func__, mreqs.imr_interface.s_addr, ifp);
+		    __func__, ntohl(mreqs.imr_interface.s_addr), ifp);
 
 		break;
 
@@ -2371,7 +2366,7 @@ inp_leave_group(struct inpcb *inp, struc
 		ims = imo_match_source(imo, idx, &ssa->sa);
 		if (ims == NULL) {
 			CTR3(KTR_IGMPV3, "%s: source 0x%08x %spresent",
-			    __func__, ssa->sin.sin_addr.s_addr, "not ");
+			    __func__, ntohl(ssa->sin.sin_addr.s_addr), "not ");
 			error = EADDRNOTAVAIL;
 			goto out_inp_locked;
 		}
@@ -2491,7 +2486,7 @@ inp_set_multicast_if(struct inpcb *inp, 
 				return (EADDRNOTAVAIL);
 		}
 		CTR3(KTR_IGMPV3, "%s: ifp = %p, addr = 0x%08x", __func__, ifp,
-		    addr.s_addr);
+		    ntohl(addr.s_addr));
 	}
 
 	/* Reject interfaces which do not support multicast. */
@@ -2869,7 +2864,7 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_A
 	group.s_addr = name[1];
 	if (!IN_MULTICAST(ntohl(group.s_addr))) {
 		CTR2(KTR_IGMPV3, "%s: group 0x%08x is not multicast",
-		    __func__, group.s_addr);
+		    __func__, ntohl(group.s_addr));
 		return (EINVAL);
 	}
 
@@ -2901,7 +2896,7 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_A
 			break;
 		RB_FOREACH(ims, ip_msource_tree, &inm->inm_srcs) {
 			CTR2(KTR_IGMPV3, "%s: visit node 0x%08x", __func__,
-			    htonl(ims->ims_haddr));
+			    ims->ims_haddr);
 			/*
 			 * Only copy-out sources which are in-mode.
 			 */

Modified: head/sys/netinet/ip_mroute.c
==============================================================================
--- head/sys/netinet/ip_mroute.c	Tue Mar 14 20:43:04 2017	(r315285)
+++ head/sys/netinet/ip_mroute.c	Tue Mar 14 20:57:54 2017	(r315286)
@@ -929,7 +929,7 @@ add_vif(struct vifctl *vifcp)
     VIF_UNLOCK();
 
     CTR4(KTR_IPMF, "%s: add vif %d laddr 0x%08x thresh %x", __func__,
-	(int)vifcp->vifc_vifi, vifcp->vifc_lcl_addr.s_addr,
+	(int)vifcp->vifc_vifi, ntohl(vifcp->vifc_lcl_addr.s_addr),
 	(int)vifcp->vifc_threshold);
 
     return 0;
@@ -1061,7 +1061,7 @@ add_mfc(struct mfcctl2 *mfccp)
     /* If an entry already exists, just update the fields */
     if (rt) {
 	CTR4(KTR_IPMF, "%s: update mfc orig 0x%08x group %lx parent %x",
-	    __func__, mfccp->mfcc_origin.s_addr,
+	    __func__, ntohl(mfccp->mfcc_origin.s_addr),
 	    (u_long)ntohl(mfccp->mfcc_mcastgrp.s_addr),
 	    mfccp->mfcc_parent);
 	update_mfc_params(rt, mfccp);
@@ -1081,7 +1081,7 @@ add_mfc(struct mfcctl2 *mfccp)
 	    !TAILQ_EMPTY(&rt->mfc_stall)) {
 		CTR5(KTR_IPMF,
 		    "%s: add mfc orig 0x%08x group %lx parent %x qh %p",
-		    __func__, mfccp->mfcc_origin.s_addr,
+		    __func__, ntohl(mfccp->mfcc_origin.s_addr),
 		    (u_long)ntohl(mfccp->mfcc_mcastgrp.s_addr),
 		    mfccp->mfcc_parent,
 		    TAILQ_FIRST(&rt->mfc_stall));
@@ -1160,7 +1160,7 @@ del_mfc(struct mfcctl2 *mfccp)
     mcastgrp = mfccp->mfcc_mcastgrp;
 
     CTR3(KTR_IPMF, "%s: delete mfc orig 0x%08x group %lx", __func__,
-	origin.s_addr, (u_long)ntohl(mcastgrp.s_addr));
+	ntohl(origin.s_addr), (u_long)ntohl(mcastgrp.s_addr));
 
     MFC_LOCK();
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201703142057.v2EKvssk043033>