From owner-svn-src-head@freebsd.org Thu Jan 26 03:03:14 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 388F8CC2BD4; Thu, 26 Jan 2017 03:03:14 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id EB233AA5; Thu, 26 Jan 2017 03:03:13 +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 mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 0C4CB1A5AD0; Thu, 26 Jan 2017 14:03:06 +1100 (AEDT) Date: Thu, 26 Jan 2017 14:03:05 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Konstantin Belousov cc: Gleb Smirnoff , 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: <20170125222632.GQ2349@kib.kiev.ua> Message-ID: <20170126133341.V1087@besplex.bde.org> References: <201701251904.v0PJ48YF061428@repo.freebsd.org> <20170125222006.GH2611@FreeBSD.org> <20170125222632.GQ2349@kib.kiev.ua> 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=c+HbeV1l c=1 sm=1 tr=0 a=Tj3pCpwHnMupdyZSltBt7Q==:117 a=Tj3pCpwHnMupdyZSltBt7Q==:17 a=kj9zAlcOel0A:10 a=0C_iaZuvqwC4Gjh4FIsA: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: Thu, 26 Jan 2017 03:03:14 -0000 On Thu, 26 Jan 2017, Konstantin Belousov wrote: > On Wed, Jan 25, 2017 at 02:20:06PM -0800, Gleb Smirnoff wrote: >> Thanks, Luiz! >> >> One stylistic nit that I missed in review: >> >> L> static int >> L> -in_difaddr_ioctl(caddr_t data, struct ifnet *ifp, struct thread *td) >> L> +in_difaddr_ioctl(u_long cmd, caddr_t data, struct ifnet *ifp, struct thread *td) >> L> { >> L> const struct ifreq *ifr = (struct ifreq *)data; >> L> const struct sockaddr_in *addr = (const struct sockaddr_in *) >> L> @@ -618,7 +618,8 @@ in_difaddr_ioctl(caddr_t data, struct if >> L> in_ifadown(&ia->ia_ifa, 1); >> L> >> L> if (ia->ia_ifa.ifa_carp) >> L> - (*carp_detach_p)(&ia->ia_ifa); >> L> + (*carp_detach_p)(&ia->ia_ifa, >> L> + (cmd == SIOCDIFADDR) ? false : true); >> >> Can we change the very last line to: >> >> (cmd == SIOCAIFADDR) ? true : false); That is not stylistic, but invert the result. Perhaps you meant to reverse the test to avoid negative logic for the result. > This is just 'cmd == SIOCAIFADDR'. Not quite. The result of a relational operator is 1 or 0 (and thus has type int), not true or false (and thus would have type bool). Unfortunately, carp has a bool infestation (the declaration of carp_attach_p() is 1 of 21 lines in netinet with bool), so int is the wrong type. It is automatically converted to bool by the prototype. 12 of the 21 lines with bools are actually mispelling of 'boolean' for IP options in in.h. 'bool' is a C type. IP options are never bools. Some of them are booleans represented as ints (never as bool since get/setsockopt() only uses int). A meta-comment in in.h describes this "bool is stored as int". Fixing the style bugs on 1 line gives another style bug -- line splitting which was to make space for the verbose spelling of a logical value. The inversion could also be written less verbosely and more clearly using the ! operator !(cmd == SIOCAIFADDR) but since the expression is so simple the ! operator can be distributed in it almost as clearly, giving !=. Then remove the parentheses. Bruce