From owner-svn-src-head@freebsd.org Fri Jan 27 06:34:55 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 79D1ECC30BA; Fri, 27 Jan 2017 06:34:55 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id 382FAE79; Fri, 27 Jan 2017 06:34:54 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from besplex.bde.org (c122-106-153-191.carlnfd1.nsw.optusnet.com.au [122.106.153.191]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id 8A054D48547; Fri, 27 Jan 2017 17:34:46 +1100 (AEDT) Date: Fri, 27 Jan 2017 17:34:45 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Gleb Smirnoff cc: Bruce Evans , Konstantin Belousov , Luiz Otavio O Souza , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r312770 - in head/sys: net netinet netinet6 In-Reply-To: <20170126215927.GL2611@FreeBSD.org> Message-ID: <20170127171655.V2822@besplex.bde.org> References: <201701251904.v0PJ48YF061428@repo.freebsd.org> <20170125222006.GH2611@FreeBSD.org> <20170125222632.GQ2349@kib.kiev.ua> <20170126133341.V1087@besplex.bde.org> <20170126215927.GL2611@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=BKLDlBYG c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=00JBZ9lbMtr6G8hdbWEA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 27 Jan 2017 06:34:55 -0000 On Thu, 26 Jan 2017, Gleb Smirnoff wrote: > On Thu, Jan 26, 2017 at 02:03:05PM +1100, Bruce Evans wrote: > B> On Thu, 26 Jan 2017, Konstantin Belousov wrote: > B> > B> > On Wed, Jan 25, 2017 at 02:20:06PM -0800, Gleb Smirnoff wrote: > B> >> Thanks, Luiz! > B> >> > B> >> One stylistic nit that I missed in review: > B> >> > B> >> L> static int > B> >> L> -in_difaddr_ioctl(caddr_t data, struct ifnet *ifp, struct thread *td) > B> >> L> +in_difaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td) > B> >> L> { > B> >> L> const struct ifreq *ifr = (struct ifreq *)data; > B> >> L> const struct sockaddr_in *addr = (const struct sockaddr_in *) > B> >> L> @@ -618,7 +618,8 @@ in_difaddr_ioctl(caddr_t data, struct if > B> >> L> in_ifadown(&ia->ia_ifa, 1); > B> >> L> > B> >> L> if (ia->ia_ifa.ifa_carp) > B> >> L> - (*carp_detach_p)(&ia->ia_ifa); > B> >> L> + (*carp_detach_p)(&ia->ia_ifa, > B> >> L> + (cmd == SIOCDIFADDR) ? false : true); > B> >> > B> >> Can we change the very last line to: > B> >> > B> >> (cmd == SIOCAIFADDR) ? true : false); > B> > B> That is not stylistic, but invert the result. Perhaps you meant to > B> reverse the test to avoid negative logic for the result. > > It uses different ioctl value, so it doesn't invert result. Instead > of !SIOCDIFADDR I want more explicit SIOCAIFADDR. Oops. So it is non-stylistic in a different way. cmd can only be SIOCDIFADDR, or one or both of SIOCAIFADDR. Than is unclear. Assuming that the original code is correct and that all 3 cases can occur, inversion would break all 3 cases, while the non-stylistic change breaks only the O_SIOCAIFADDR case. Since there can be more than 2 cases and it isn't clear that there are at most 3, any boolean test on 1 of the cases is going to be unclear. Positive logic will be clearer, but that requires comparison with 2 cases. The current code use negative logic to select these 2 cases as the complement of the other case. Bruce