From owner-freebsd-net@FreeBSD.ORG Sat Nov 4 02:46:35 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 1C6A316A407 for ; Sat, 4 Nov 2006 02:46:35 +0000 (UTC) (envelope-from antinvidia@gmail.com) Received: from nf-out-0910.google.com (nf-out-0910.google.com [64.233.182.187]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3CD9643D53 for ; Sat, 4 Nov 2006 02:46:32 +0000 (GMT) (envelope-from antinvidia@gmail.com) Received: by nf-out-0910.google.com with SMTP id i2so1676855nfe for ; Fri, 03 Nov 2006 18:46:31 -0800 (PST) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:references; b=bnh0RuuAkBSY3rj2roaJ/ykXNmUDCq36kNX1aUusZ7zU7DHM0PFFCI5QPLjLh/ZDgsCocK9C3572ihV0beHeBec8aRBr2k22FusOECVJ8ymhQiUpVZch380c3662gs8b8w9HyXkLcdZoJ91V61xBLnoy/Je+IPoJgZ1G5AiLofg= Received: by 10.49.49.4 with SMTP id b4mr2297220nfk.1162608390912; Fri, 03 Nov 2006 18:46:30 -0800 (PST) Received: by 10.49.37.15 with HTTP; Fri, 3 Nov 2006 18:46:30 -0800 (PST) Message-ID: Date: Sat, 4 Nov 2006 02:46:30 +0000 From: MQ To: "Brooks Davis" In-Reply-To: <20061103141732.GA87515@lor.one-eyed-alien.net> MIME-Version: 1.0 References: <20061102142543.GC70915@lor.one-eyed-alien.net> <20061103141732.GA87515@lor.one-eyed-alien.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline X-Content-Filtered-By: Mailman/MimeDel 2.1.5 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: Sat, 04 Nov 2006 02:46:35 -0000 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 may > > >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 all the occurrences consistent. Maybe a better way is use a macro to make that clear? #define IPV4_ADDRSZ (4 * sizeof "123") char buf[IPV4_ADDRSZ]; 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.