Date: Tue, 24 Nov 1998 21:35:27 +1100 From: Bruce Evans <bde@zeta.org.au> To: archie@whistle.com, bde@zeta.org.au Cc: dillon@apollo.backplane.com, freebsd-current@FreeBSD.ORG, grog@lemis.com, rnordier@nordier.com Subject: Re: snprintf() in the kernel Message-ID: <199811241035.VAA30568@godzilla.zeta.org.au>
next in thread | raw e-mail | index | archive | help
>> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199811241035.VAA30568>