From owner-freebsd-net@FreeBSD.ORG Sun Nov 5 01:18:56 2006 Return-Path: X-Original-To: freebsd-net@freebsd.org Delivered-To: freebsd-net@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id CADDC16A49E for ; Sun, 5 Nov 2006 01:18:56 +0000 (UTC) (envelope-from brooks@lor.one-eyed-alien.net) Received: from sccmmhc91.asp.att.net (sccmmhc91.asp.att.net [204.127.203.211]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4510843D46 for ; Sun, 5 Nov 2006 01:18:56 +0000 (GMT) (envelope-from brooks@lor.one-eyed-alien.net) Received: from lor.one-eyed-alien.net ([12.207.12.9]) by sccmmhc91.asp.att.net (sccmmhc91) with ESMTP id <20061105011853m9100ekcoee>; Sun, 5 Nov 2006 01:18:53 +0000 Received: from lor.one-eyed-alien.net (localhost [127.0.0.1]) by lor.one-eyed-alien.net (8.13.8/8.13.8) with ESMTP id kA51Io3V007728; Sat, 4 Nov 2006 19:18:50 -0600 (CST) (envelope-from brooks@lor.one-eyed-alien.net) Received: (from brooks@localhost) by lor.one-eyed-alien.net (8.13.8/8.13.8/Submit) id kA51Ioml007727; Sat, 4 Nov 2006 19:18:50 -0600 (CST) (envelope-from brooks) Date: Sat, 4 Nov 2006 19:18:49 -0600 From: Brooks Davis To: MQ Message-ID: <20061105011849.GB6669@lor.one-eyed-alien.net> References: <20061102142543.GC70915@lor.one-eyed-alien.net> <20061103141732.GA87515@lor.one-eyed-alien.net> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="f2QGlHpHGjS2mn6Y" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.11 Cc: freebsd-net@freebsd.org Subject: Re: Reentrant problem with inet_ntoa in the kernel X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 05 Nov 2006 01:18:57 -0000 --f2QGlHpHGjS2mn6Y Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Nov 04, 2006 at 02:46:30AM +0000, MQ wrote: > 2006/11/3, Brooks Davis : > > > >On Fri, Nov 03, 2006 at 09:46:47AM +0000, MQ wrote: > >> 2006/11/2, Brooks Davis : > >> > > >> >On Thu, Nov 02, 2006 at 08:26:27AM +0000, . wrote: > >> >> Hi, > >> >> > >> >> I am confused by the use of inet_ntoa function in the kernel. > >> >> > >> >> The function inet_ntoa in the /sys/libkern/inet_ntoa.c uses a static > >> >array > >> >> static char buf[4 * sizeof "123"]; > >> >> to store the result. And it returns the address of the array to the > >> >caller. > >> >> > >> >> I think this inet_ntoa is not reentrant, though there are several > >> >functions > >> >> calling it. If two functions call it simultaneously, the result will > >be > >> >> corrupted. Though I haven't really encountered this situation, it m= ay > >> >occur > >> >> someday, especially when using multi-processors. > >> >> > >> >> There is another reentrant version of inet_ntoa called inet_ntoa_r = in > >> >the > >> >> same file. It has been there for several years, but just used by > >ipfw2 > >> >for > >> >> about four times in 7-CURRENT. In my patch, I replaced all the calls > >to > >> >> inet_ntoa with calls to inet_ntoa_r. > >> >> > >> >> By the way, some of the original calls is written in this style: > >> >> strcpy(buf, inet_ntoa(ip)) > >> >> The modified code is written in this style > >> >> inet_ntoa_r(ip, buf) > >> >> This change avoids a call to strcpy, and can save a little time. > >> >> > >> >> Here is the patch. > >> >> > >> > > >http://people.freebsd.org/~delphij/misc/patch-itoa-by-nodummy-at-yeah-net > >> >> > >> >> I've already sent to PR(kern/104738), but got no reply, maybe it > >should > >> >be > >> >> discussed here first? > >> > > >> >I've got to agree with other posters that the stack variable > >allocations > >> >are ugly. What about extending log and printf to understand ip4v > >> >addresses? That's 90% of the uses and the others appears to have > >> >buffers already. > >> > > >> >-- Brooks > >> > > >> > > >> >Ugly? Why? Don't you use local variables in your sources? > > > >The particular definition used is excedingly ugly. At a minimum there > >needs to be a define or a constant "16" for the lenght rather than the > >4*sizeof("123") nonsense. > > > >-- Brooks > > > > > > > Oh, I see. This kind of definition is really annoying, and hard to keep a= ll > the > occurrences consistent. Maybe a better way is use a macro to make that > clear? >=20 > #define IPV4_ADDRSZ (4 * sizeof "123") > char buf[IPV4_ADDRSZ]; >=20 > This "ugly" definition comes from inet_ntoa() in /sys/libkern/inet_ntoa.c, > I just copied the style without too much consideration, it's my fault. I'd just use 16. The inet_ntoa function is frankly inane. It attempts to support chars that aren't 8 bits which would break so much stuff it isn't funny. -- Brooks --f2QGlHpHGjS2mn6Y Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (FreeBSD) iD8DBQFFTTv5XY6L6fI4GtQRAnIjAJ428EtXz7lIYgbYl+3TiXlrXx23eQCgtofE yKK4poW//ljNzoV46uBC5ik= =+uSP -----END PGP SIGNATURE----- --f2QGlHpHGjS2mn6Y--