From owner-freebsd-current@FreeBSD.ORG Sun Jul 29 09:38:32 2012 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8625D106568C for ; Sun, 29 Jul 2012 09:38:32 +0000 (UTC) (envelope-from luigi@onelab2.iet.unipi.it) Received: from onelab2.iet.unipi.it (onelab2.iet.unipi.it [131.114.59.238]) by mx1.freebsd.org (Postfix) with ESMTP id 1FBAE8FC17 for ; Sun, 29 Jul 2012 09:38:31 +0000 (UTC) Received: by onelab2.iet.unipi.it (Postfix, from userid 275) id B9E007300A; Sun, 29 Jul 2012 11:58:33 +0200 (CEST) Date: Sun, 29 Jul 2012 11:58:33 +0200 From: Luigi Rizzo To: "Bjoern A. Zeeb" Message-ID: <20120729095833.GB80946@onelab2.iet.unipi.it> References: <20120725155211.GA33971@onelab2.iet.unipi.it> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i Cc: current@freebsd.org Subject: Re: RFC: libkern version of inet_ntoa_r X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 Jul 2012 09:38:32 -0000 On Sat, Jul 28, 2012 at 10:14:10PM +0000, Bjoern A. Zeeb wrote: > On Wed, 25 Jul 2012, Luigi Rizzo wrote: > > >During some ipfw/dummynet cleanup i noticed that the libkern version of > >inet_ntoa_r() is missing the buffer size argument that is present in > >the libc counterpart. > > > >Any objection if i fix it ? > > And why exactly would you need it? What does libc do with it? Render > partial IPv4 addresses? Thanks for raising the issue because things are actually simpler than i thought. In general, having a function with same name and different arguments/behaviour in the kernel and userspace is annoying and should be avoided/fixed if possible. We have to live with malloc/free as they are too widely used to suggest a change, but this inet_ntoa_r() is very seldom used. In fact, the libc version is never used in HEAD, stable/9 and stable/8 and only the libkern version has a small number of clients (see below). Given that libkern has inet_ntop, with the same arguments of the userspace version, we'd be much better off with the following course of action: 1. replace calls to inet_ntoa_r() with inet_ntop() This needs to be done only in the kernel, because the libc version is never used at least in the source tree (maybe some port does). 2. nuke inet_ntoa_r() from libkern 3. nuke inet_ntoa_r() from libc The discussion on the cost of the extra argument is not very relevant here, because the function is intrinsically slow and called on slow paths (involves an snprintf, and likely some device I/O downstream). cheers luigi # grep -r net_ntoa_r . | grep -Ev pristine ./include/arpa/inet.h:#define inet_ntoa_r __inet_ntoa_r ./include/arpa/inet.h:char *inet_ntoa_r(struct in_addr, char *buf, socklen_t size); ./lib/libc/inet/Symbol.map: __inet_ntoa_r; ./lib/libc/inet/Symbol.map: inet_ntoa_r; ./lib/libc/inet/inet_ntoa.c:inet_ntoa_r(struct in_addr in, char *buf, socklen_t size) ./lib/libc/inet/inet_ntoa.c:__weak_reference(__inet_ntoa_r, inet_ntoa_r); ./lib/libc/net/Makefile.inc: inet.3 inet_network.3 inet.3 inet_ntoa.3 inet.3 inet_ntoa_r.3\ ./lib/libc/net/inet.3:.Nm inet_ntoa_r , ./lib/libc/net/inet.3:.Fo inet_ntoa_r ./lib/libc/net/inet.3:.Fn inet_ntoa_r ./sys/libkern/inet_ntoa.c:inet_ntoa_r(struct in_addr ina, char *buf) ./sys/net/flowtable.c: inet_ntoa_r(ssin->sin_addr, saddr); ./sys/net/flowtable.c: inet_ntoa_r(dsin->sin_addr, daddr); ./sys/net/flowtable.c: inet_ntoa_r(*(struct in_addr *) &dsin->sin_addr, daddr); ./sys/net/flowtable.c: inet_ntoa_r(*(struct in_addr *) &hashkey[2], daddr); ./sys/net/flowtable.c: inet_ntoa_r(*(struct in_addr *) &hashkey[1], saddr); ./sys/net/if_llatbl.c: inet_ntoa_r(sin->sin_addr, l3s); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, src); ./sys/netinet/ipfw/ip_fw_dynamic.c: inet_ntoa_r(da, dst); ./sys/netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip->ip_src, src); ./sys/netinet/ipfw/ip_fw_log.c: inet_ntoa_r(ip->ip_dst, dst); ./sys/netinet/in.h:char *inet_ntoa_r(struct in_addr ina, char *buf); /* in libkern */ ./sys/netinet/in_pcb.c: inet_ntoa_r(inc->inc_laddr, laddr_str); ./sys/netinet/in_pcb.c: inet_ntoa_r(inc->inc_faddr, faddr_str); ./sys/netinet/tcp_subr.c: inet_ntoa_r(inc->inc_faddr, sp); ./sys/netinet/tcp_subr.c: inet_ntoa_r(inc->inc_laddr, sp); ./sys/netinet/tcp_subr.c: inet_ntoa_r(ip->ip_src, sp); ./sys/netinet/tcp_subr.c: inet_ntoa_r(ip->ip_dst, sp); > ` I need it because i would like to compile parts of the kernel in userspace, and having a kernel function with the same name and different arguments from of a libc function is annoying. I can accept that > -- > Bjoern A. Zeeb You have to have visions! > Stop bit received. Insert coin for new address family.