Re: question about change in inet_ntoa.c

On Tue, 26 Feb 2008, ithilgore wrote:

Giorgos Keramidas wrote:
On 2008-02-23 02:37, ithilgore <ithilgore.fbsd@xxxxxxxxx> wrote:

ithilgore wrote:

I was looking at the differences between some old FreeBSD code
and the one of 7.0-RC1 and was wondering about a change
in inet_ntoa.c

/***** 7.0-RC1 **************/

sprintf(buf, "%d.%d.%d.%d",
ucp[0] & 0xff,
ucp[1] & 0xff,
ucp[2] & 0xff,
ucp[3] & 0xff);

This version in libkern is best (except `buf' is static, so it is not
reentrant and thus quite broken). ucp[N] is unsigned char, but masking
with 0xff is needed to support the (unsupported) machines with more
than 8 bits in their chars. On normal machines with 8-bit chars, the
compiler will optimize away the masks so their only cost is increased
portability of the source code. POSIX now requires 8-bit chars, but
didn't when the above was written. %d is good enough after masking,
since the result is nonnegative and <= INT_MAX. %u would be a style
bug except on exotic machines with sizeof(char) == sizeof(int), since
the default promotions normally turn all the numeric printf args into ints (not u_ints) thanks to C90's broken "value-preserving" promotion
rules.

/****** 4.11-RELEASE ***********/


static const char fmt[] = "%u.%u.%u%u";
if ((size_t)snprintf(dst, size, fmt, src[0], src[1], src[2], src[3])
>= size) {

This is actually in somewhere/inet_ntop.c:inet_ntop4().

Bugs in this version include:
- benign type mismatch between %u and the promoted args except on exotic
machines
- broken on exotic machines. UCHAR_MAX can be arbitarily large. Then
large values will e put in the string. sprintf() could then overrun
the buffer, but using snprintf() avoids that.
- bogus cast of snprintf's return value.

-current has a different version in libc, without the bogus cast. Many
files in libc/net including inet_ntop.c moved to libc/inet, and some
nearby FreeBSD cleanups were lost (ISC cleaned things up differently
and not so well in one case that I looked at).

....
....

Was there a specific purpose of changing the more easy and simple way
of %u instead of the combination of %d and and-ing with 0xff ??
It essentially gives the same result but increases overhead (i think) in the more
recent version.

I think the libkern version was written later, and it is better because
its author knows what is portable. It's also much simpler. After and-ing
with 0xff, we know the range of the values and don't have to understand
UCHAR_MAX to know that our code is only broken on unsupported/exotic
machines.

On the other hand, in version 4.11 RELEASE in /usr/src/lib/libc/net/inet_ntoa.c &
inet_ntop.c (actually it is inet_ntop.c code but with the same functionality)

But hard to find if you are looking for inet_ntoa.c.

There is also libstand/inet_ntoa.c. It has different bugs and style bugs.
At least in old versions, it combines the worst features of libkern and
libc (static buffer; uses sprintf() and thus can overrun on exotic machines;
otherwise mainly style bugs like libc).

which is
called by inet_ntoa there is :

static const char *
inet_ntop4(src, dst, size)
const u_char *src;
char *dst;
size_t size;
{
static const char fmt[] = "%u.%u.%u.%u";

if ((size_t)snprintf(dst, size, fmt, src[0], src[1], src[2], src[3])
>= size) {

Another dubious cleanup in the -current version is to first snprintf() to a
static buffer of length 16 and copy to the caller's buffer from there.
snprintf() makes that unnecessary unless the API requires not touching the
caller's buffer on error.

errno = ENOSPC;
return (NULL);
}
return (dst);
}

Bruce
_______________________________________________
freebsd-net@xxxxxxxxxxx mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscribe@xxxxxxxxxxx"

Relevant Pages

  • Re: question about change in inet_ntoa.c
    ... ucpis unsigned char, but masking ... On normal machines with 8-bit chars, ... the buffer, ... It has different bugs and style bugs. ...
    (freebsd-net)
  • Re: How to develop a random number generation device
    ... OS/2 did a pretty good job of preventing anything ... to stop all buffer overruns, but you can still guard against a lot of ... computing when they kluged up x86 and Windows. ... Key0 on the old 360 and 370 series machines if you read the right ...
    (sci.electronics.design)
  • Re: [PATCH 4/4] ACPI PCI slot detection driver
    ... Greg KH wrote: ... My main concern is that BIOS vendors will not fix these bugs, ... And then there's the machines with duplicate slot names, ... non-hotplug slots named the same as the hotplug slots, ...
    (Linux-Kernel)
  • Re: [PATCH 4/4] ACPI PCI slot detection driver
    ... My main concern is that BIOS vendors will not fix these bugs, ... And then there's the machines with duplicate slot names, ... machine, one being hotplug, the other non-hotplug. ...
    (Linux-Kernel)
  • Re: [PATCH 4/4] ACPI PCI slot detection driver
    ... My main concern is that BIOS vendors will not fix these bugs, ... before shipment to make sure that the BIOS info is up to date... ... And then there's the machines with duplicate slot names, ... non-hotplug slots named the same as the hotplug slots, ...
    (Linux-Kernel)