Date: Sun, 5 Nov 2006 02:58:36 +0000 From: MQ <antinvidia@gmail.com> To: "Brooks Davis" <brooks@one-eyed-alien.net> Cc: freebsd-net@freebsd.org Subject: Re: Reentrant problem with inet_ntoa in the kernel Message-ID: <be0088ce0611041858h32a514cche914fa964dee2066@mail.gmail.com> In-Reply-To: <20061105011849.GB6669@lor.one-eyed-alien.net> References: <be0088ce0611020026y4fe07749pd5a984f8744769b@mail.gmail.com> <20061102142543.GC70915@lor.one-eyed-alien.net> <be0088ce0611030146u5e97e08cmbd36e94d772c8a94@mail.gmail.com> <20061103141732.GA87515@lor.one-eyed-alien.net> <be0088ce0611031846l469b096bl536fec1d243da13f@mail.gmail.com> <20061105011849.GB6669@lor.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
2006/11/5, Brooks Davis <brooks@one-eyed-alien.net>: > > On Sat, Nov 04, 2006 at 02:46:30AM +0000, MQ wrote: > > 2006/11/3, Brooks Davis <brooks@one-eyed-alien.net>: > > > > > >On Fri, Nov 03, 2006 at 09:46:47AM +0000, MQ wrote: > > >> 2006/11/2, Brooks Davis <brooks@one-eyed-alien.net>: > > >> > > > >> >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. > > 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 > > > Yes, you are right. Refer to the ISO/IEC C standard, the characters that may used by an ipv4 address are members of the basic character sets defined by the standard. And the char type should be large enough to contain any member of the basic execution character set. So just defining the array size to 16 is enough. And the original writer considered to much(or the C standard of that time didn't make such clear rules on the character set).
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?be0088ce0611041858h32a514cche914fa964dee2066>