On Wed, 5 Jul 2000, Frank van Maarseveen wrote:
> On Wed, Jul 05, 2000 at 02:08:49PM +0200, Olaf Titz wrote:
> > The buffer allocation works as follows: every conversion routine has
> > its own chain (salloc_t) of buffers (scope_buf_t). Whenever a buffer
> > is needed, it looks for a free one in the chain. Each buffer is tagged
> > with the return address of the caller (here called "scope"). A buffer
> Very clever. But probably too fragile for various reasons. A ring of
> say 30 buffers might be less fragile and simpler.
>
> As others have pointed out a new format specifier would be the best
> solution. Extending gcc function attributes for checking the format
> string would be necessary. IMHO the time is right to push this into
> egcs/gcc, at least for IPv6 addresses.
My opinion now is that we should kill the version of in_ntoa that doesn't
take a buf, and force everyone to declare automatics. The two standard
uses of in_ntoa are:
Orig: Fixed:
char a[18];
printk(..., in_ntoa(foo)); printk(..., in_ntoa(foo, a));
and and
strcpy(buf, in_ntoa(foo)); in_ntoa(foo, buf);
Aside from the whole multiple printk args nuisance, is there anything that
makes in_ntoa threadsafe? Like always getting called under a network lock?
I don't think so. Since it looks like its getting called to fill in
protocol headers in NFS and the like, it could be dangerous.
There seem to be about 30 instances of ip_ntoa, none of ip_ntoa2, and
about 60 of NIPQUAD. NIPQUAD is a magic macro that expands to a single
macro and doesn't have a nice IPv6 analog and HIPQUAD is currently broken
wrt endianness so I'd vote for nixing them all in favor of a single
function that works like ip_ntoa2. NIPQUAD always appears as a *print*
arg, so replacement would be as above.
BTW, the v6 code currently has stuff like this in it (icmp.c):
printk(KERN_DEBUG "ICMPv6 checksum failed
[%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x >
%04x:%04x:%04x:%04x:%04x:%04x:%04x:%04x]\n",
ntohs(saddr->in6_u.u6_addr16[0]),
ntohs(saddr->in6_u.u6_addr16[1]),
ntohs(saddr->in6_u.u6_addr16[2]),
ntohs(saddr->in6_u.u6_addr16[3]),
ntohs(saddr->in6_u.u6_addr16[4]),
ntohs(saddr->in6_u.u6_addr16[5]),
ntohs(saddr->in6_u.u6_addr16[6]),
ntohs(saddr->in6_u.u6_addr16[7]),
ntohs(daddr->in6_u.u6_addr16[0]),
ntohs(daddr->in6_u.u6_addr16[1]),
ntohs(daddr->in6_u.u6_addr16[2]),
ntohs(daddr->in6_u.u6_addr16[3]),
ntohs(daddr->in6_u.u6_addr16[4]),
ntohs(daddr->in6_u.u6_addr16[5]),
ntohs(daddr->in6_u.u6_addr16[6]),
ntohs(daddr->in6_u.u6_addr16[7]));
Oh, and I'm guessing the ntohl below is wrong, Alan?
./drivers/net/wan/sdla_chdlc.c:
in_ntoa(ntohl(chdlc_priv_area->IP_address)));
./drivers/net/wan/sdla_chdlc.c:
in_ntoa(ntohl(chdlc_priv_area->IP_address)));
-- "Love the dolphins," she advised him. "Write by W.A.S.T.E.."- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Fri Jul 07 2000 - 21:00:18 EST