From owner-freebsd-current  Tue Nov 24 02:36:00 1998
Return-Path: <owner-freebsd-current@FreeBSD.ORG>
Received: (from majordom@localhost)
          by hub.freebsd.org (8.8.8/8.8.8) id CAA04031
          for freebsd-current-outgoing; Tue, 24 Nov 1998 02:36:00 -0800 (PST)
          (envelope-from owner-freebsd-current@FreeBSD.ORG)
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.15.68.22])
          by hub.freebsd.org (8.8.8/8.8.8) with ESMTP id CAA04023
          for <freebsd-current@FreeBSD.ORG>; Tue, 24 Nov 1998 02:35:49 -0800 (PST)
          (envelope-from bde@godzilla.zeta.org.au)
Received: (from bde@localhost)
	by godzilla.zeta.org.au (8.8.7/8.8.7) id VAA30568;
	Tue, 24 Nov 1998 21:35:27 +1100
Date: Tue, 24 Nov 1998 21:35:27 +1100
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199811241035.VAA30568@godzilla.zeta.org.au>
To: archie@whistle.com, bde@zeta.org.au
Subject: Re: snprintf() in the kernel
Cc: dillon@apollo.backplane.com, freebsd-current@FreeBSD.ORG, grog@lemis.com,
        rnordier@nordier.com
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

>> It would be better without any strncpy() patches.
>
>Well, it's certainly easy enough to take them out.. however, some
>are bug fixes...
>
>For example, if a function takes a string argument, when can you
>assume an upper bound on how long the string is?

strncpy() already takes a length arg.  Changing from strncpy() to
snprintf() just wastes space and time unless the value returned by
snprintf() is actually checked and cases where the string doesn't
fit are actually handled.

>Of the strncpy() replacements, there are three categories:
>
>- "Possibly unterminated string", where one is required:
>
>    netatm/spans/spans_print.c

It's interesting that it doesn't just printf everything (using separate
printfs because spans-addr_print() returns a pointer to static storage).
Most of the strncpy's seem to be to copy the static storage to local
storage.  All buffers for this seem to have size 80, so there are no
truncation problems.

>    netatm/uni/uniarp_cache.c
>    pc98/pc98/diskslice_machdep.c
>    pci/pci_compat.c
>
>- "Simplification" (eg, replacing constants like "16" with sizeof())
>  with otherwise no functional effect (including strncpy()'s zero'ing
>  out of the buffer):
>
>    i386/ibcs2/ibcs2_stat.c
>    i386/ibcs2/ibcs2_xenix.c
>    i386/linux/linux_misc.c

These depend on a previous bzero() to zero out the buffer (and any padding
in the struct).  The strncpy()'s of course should have used sizeof() instead
of literal constants.  The explicit \0 termiation could have been avoided
in ibcs2_stat.c by subtracting 1 from the sizes.  This is already done in
ibcs2_xenix.c and linux_misc.c, but it is hard to see because sizeof() is 
not used and the subtraction is done at code-write time.

>    netinet/ip_divert.c
>
>- "Gratuitous" (?)
>
>    alpha/tc/tcds.c
>    dev/dpt/dpt_control.c
>    kern/subr_devstat.c

This is certainly sloppy about null termination.  It depends on device
name lengths being shorter than 15.  Most device names have length 2
(the others are too long :-) so there are no problems in practice.

>    netatm/atm_aal5.c
>    netatm/atm_socket.c
>
>Tell me which (or all) of these you don't want and I'll take them out;
>however my instinct would say to keep the first two sets.

>If it's performance you're thinking about, my general assumption is
>that string manipulation in the kernel is uncommon, but that could
>be wrong.

strncpy() seems to be only twice as fast as snprintf() in the kernel
(when both are in the L1 cache and strncpy() doesn't have to do much
padding).

Bruce

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message