Re: CVS commit: src/external/gpl3
On Tue, 11 Aug 2015, David Laight wrote: The system should probably clean 'turds' from both /tmp and /var/tmp. That would be surprising, at least to me. I rely on /var/tmp/vi.recover being persistent, and I often use /var/tmp as a place to stash work in progress that should survive a reboot. --apb (Alan Barrett)
Re: CVS commit: src/share/misc
On Sat, 25 Apr 2015, rod...@netbsd.org wrote: I'm responding to the recent posts ITT, because it seems there's some misunderstanding: I had no idea what ITT meant, until I looked it up. (ITT = "in this thread", apparently). 1) What has been committed is no more "offensive" than the existing material in wtf and fortune. Please, review fortune's data files if any doubts exist; 2) wtf and fortune have existed in source since $TIME without any uproar; 3) There aren't any rules documenting appending entries to either; 4) We don't tend to make up rules as we go along committing; We have a rule or at least a convention that things that are likely to be controversial should be discussed first. If you don't realise that something is controversial, and commit without discussion, then the appropriate response is to engage in discussion as soon as you learn that the issue was controversial. Continuing without discussion is not appropriate. I think that NetBSD's acronyms file should be for acronyms that ordinary people are likely to encounter in ordinary situations; not for acronyms used in some small subculture. --apb (Alan Barrett)
Re: CVS commit: src/share/misc
On Fri, 24 Apr 2015, Taylor R Campbell wrote: Modified Files: src/share/misc: acronyms acronyms-o.real Log Message: Per discussion with board@, remove inappropriate, hostile acronyms. Thank you. --apb (Alan Barrett)
Re: CVS commit: src/external/cddl/osnet
On Sat, 11 Apr 2015, Taylor R Campbell wrote: Date: Sat, 11 Apr 2015 15:12:01 +1000 from: matthew green "Taylor R Campbell" writes: When modifying, double-check that libnvpair.so defines no xdr_* symbols, only _solaris_xdr_*. (XXX Put this note somewhere...) somewhere == doc/HACKS. It's not really a hack, though. This is just the way we have to make colliding symbols not collide. What we don't have is a way to automate it so you don't have to write `#define xdr_foo _solaris_xdr_foo', or so the toolchain will notify you if you forgot to. You could write a test that uses nm|awk to check that all externally visible symbols have some desired prefix. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/time
On Mon, 06 Apr 2015, Brian Ginsbach wrote: Module Name:src Committed By: ginsbach Date: Mon Apr 6 14:38:22 UTC 2015 Modified Files: src/lib/libc/time: strptime.3 strptime.c Log Message: Add UTC as a synonym for GMT (%Z). [from FreeBSD] The %z format (which is distinct from %Z) already accepts "GMT" and "UT". If we are going to accept "UTC" as well, which I think is a good idea, then please allow both %z and %Z to accept all three of UT, UTC, and GMT. --apb (Alan Barrett)
Re: CVS commit: src/usr.sbin/service
On Wed, 01 Apr 2015, Adrian Steinmann wrote: Please could this be fixed to use shell quoting in a safe way. OK, if that'll unstall the pullup-7. I don't know. Are you implying that the /etc/rc.d/ system supports space in filenames? No, I am implying that somebody could create a file whose name contains a space. Whether or not it's supported is separate from whether or not the service(8) script does strange things when it happens. --apb (Alan Barrett)
Re: CVS commit: src/usr.sbin/service
On Fri, 27 Mar 2015, Adrian Steinmann wrote: Modified Files: src/usr.sbin/service: service Please could this be fixed to use shell quoting in a safe way. For example ... +_rc_files() { -local dir -for dir in ${rc_directories}; do -[ -d ${dir} ] && ls -P1 ${dir} 2>/dev/null +local _d _f +for _d in ${rc_directories}; do +if [ -d $_d ]; then +for _f in $_d/*; do [ -f $_f -a -x $_f ] && echo $_f; done +fi should be ... if [ -d "$_d" ]; then for _f in "$_d"/*; do [ -f "$_f" -a -x "$_f" ] && echo "$_f" done fi --apb (Alan Barrett)
Re: CVS commit: src
On Sat, 07 Mar 2015, Martin Husemann wrote: On Sat, Mar 07, 2015 at 08:13:17AM -0500, Greg Troxel wrote: What's the set of computers that one has to use LEGACY on? Anything that has no PCI. Could we rename LEGACY to something more descriptive? The problem with the name "LEGACY" is that it leaves people wondering whether or not particular hardware is old enough to be classified as "legacy". If the test is "Does it have an ISA bus without a PCI bus?" then I'd suggest a name like ISA_NO_PCI. This is another case where prior discussion would have been a good idea. --apb (Alan Barrett)
Re: CVS commit: src
On Fri, 02 Jan 2015, Christos Zoulas wrote: Log Message: Implement DIOCGMEDIASIZE and DIOCGSECTORSIZE from FreeBSD. This needs compat32 handling, at least for the u_int arg to DIOCGSECTORSIZE. Why not make it a fixed size, like uint32_t, so compat32 handling will not be needed? I think it was made u_int to match previous art by FreeBSD. Can you please explain why it needs compat32 handling? Sorry, it doesn't need compat32 handling, because all existing NetBSD platforms have 32-bit int, and all exiting NetBSD platforms have 64-bit off_t. --apb (Alan Barrett)
Re: CVS commit: src
On Mon, 29 Dec 2014, Michael van Elst wrote: Log Message: Implement DIOCGMEDIASIZE and DIOCGSECTORSIZE from FreeBSD. This needs compat32 handling, at least for the u_int arg to DIOCGSECTORSIZE. Why not make it a fixed size, like uint32_t, so compat32 handling will not be needed? --apb (Alan Barrett)
Re: CVS commit: src
On Mon, 17 Nov 2014, Masao Uebayashi wrote: How about adding a new make variable CONFIGOPTS, which is passed to config(1)? This name is similar to COPTS and works similarly too. Yes, that could work. --apb (Alan Barrett)
Re: CVS commit: src
On Mon, 17 Nov 2014, Martin Husemann wrote: On Sun, Nov 16, 2014 at 06:08:13AM +, Masao Uebayashi wrote: Modified Files: src: build.sh Log Message: build.sh mkernels: Build all kernels in modular build Sorry to be slow here, but: this "modular" obviously differs from modular kernels (as in: have "options MODULAR") - so I guess the name is not a good one. I actually have no clue what it is supposed to mean. Also, why does it need a new build.sh action? Why can't the choice of how to build kernels be triggered by a variable in a Makefile or in the environment or in the kernel configuration? Was this discussed anywhere? --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/config
On Thu, 30 Oct 2014, Taylor R Campbell wrote: It seems to me that while depending on ordering for definitions, files, &c., may be no good, for selections the language of include "GENERIC" no options DIAGNOSTIC no agp* is still valuable. I don't mind how it's implemented, but my main concern is that I want an example like the above to "work". I want it to mean: my kernel should be based on GENERIC with some changes; whether or not "options DIAGNOSTIC" is present in GENERIC, my kernel should not have "options DIAGNOSTIC"; and whether or not anything related to agp is present in GENERIC, my kernel should not have any agp devices (and should not have anything that depends on agp). I do not want error messages like "options DIAGNOSTIC was already off, so you are not allowed to try to turn it off again" or "there were no agp devices, so you are not allowed to try to remove agp devices". So config(1) ought to choose whatever is the last yes/no answer for a selection in order to decide what things are really enabled or disabled, and then process dependencies recursively from there, rather than incrementally processing dependencies as the parser makes progress. That sounds as though it would do what I want. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/config
On Thu, 30 Oct 2014, Masao Uebayashi wrote: What do you expect by doing: options FOO no options FOO options FOO I expect it to be equivalent to just one "options FOO". The "no options FOO" in line 2 should cancel the "options FOO" in line 1, and then the "options FOO" in line 3 should put it back. In the cases that I care about, the "options" and "no options" lines will be in different files, referenced via "include" directives. --apb (Alan Barrett)
Re: CVS commit: othersrc/external/bsd/multigest
On Tue, 28 Oct 2014, Alistair Crooks wrote: >+.ifndef PRINTOBJDIR >+PRINTOBJDIR=${MAKE} -V .OBJDIR >+.endif At least for NetBSD's make(1), you need ${MAKE} -V '${.OBJDIR}' to get the recursively-expanded value of .OBJDIR. ${MAKE} -V .OBJDIR would print the unexpanded value. Oh, you'd better fix it in NetBSD's bsd.own.mk then, which was where I took the definition above from. It probably works because .OBJDIR is magic, and is unlikely to need recursive expansion. --apb (Alan Barrett)
Re: CVS commit: othersrc/external/bsd/multigest
On Tue, 28 Oct 2014, Alistair G. Crooks wrote: Modified Files: othersrc/external/bsd/multigest/bin: Makefile othersrc/external/bsd/multigest/lib: Makefile +.ifndef PRINTOBJDIR +PRINTOBJDIR=${MAKE} -V .OBJDIR +.endif At least for NetBSD's make(1), you need ${MAKE} -V '${.OBJDIR}' to get the recursively-expanded value of .OBJDIR. ${MAKE} -V .OBJDIR would print the unexpanded value. --apb (Alan Barrett)
Re: CVS commit: src/distrib/sets
On Fri, 24 Oct 2014, Jeff Rizzo wrote: Modified Files: src/distrib/sets: join.awk Log Message: Back out previous until it can be fixed - it was causing all sets to contain all files, which made a full build of all arches over 150GB! Sorry, I'll try to fix it soon, but it may have to wait a few days. Meanwhile, backing out the problematic revision should be fine. --apb (Alan Barrett)
Re: CVS commit: src/bin/sh
On Sun, 19 Oct 2014, Christos Zoulas wrote: There are some TOCTOU races in this code, where something about the file could change in between the stat() and the open(). Well, we could try to open without O_CREAT first, for device nodes it should succeed, if it fails do the O_EXCL thingy. I think open has enough flags. Yes, that would probably work, and it's easier than my ideas. --apb (Alan Barrett)
Re: CVS commit: src/bin/sh
On Wed, 15 Oct 2014, Christos Zoulas wrote: Modified Files: src/bin/sh: redir.c Log Message: PR/48201: Miwa Susumu: Fix set -C (no clobber) for POSIX; from FreeBSD Can't use O_EXCL because of device nodes; also truncate. There are some TOCTOU races in this code, where something about the file could change in between the stat() and the open(). Some ideas: 1. Keep the new code, with its races, but also verify that st_dev and st_ino values remain unchanged between the stat() before opening the file, and fstat() after opening the file. 2. Try open() with O_EXCL first, and fall back to racy code with stat() only if the first open(O_EXCL) fails. Also use fstat() to check that st_dev/st_ino do not change. 3. Invent one or more open(2) flags, such as O_SPECIAL for "must be a device or other special file, must not be a plain file or a directory". First try open(O_EXCL), and if that fails then try open(O_SPECIAL). --apb (Alan Barrett)
Re: CVS commit: src/sys/fs/puffs
On Wed, 15 Oct 2014, David Laight wrote: Consider what happens if you write: if (error) DPRINTF((...)); else fubar(); When DPRINTF() expands 'if (xxx) yyy' it all goes horribly wrong. That's why I changed it to do { if (xxx) yyy; } while (0) a week or two ago. (That change was not pulled up to netbsd-7, although other changes made around the same time were pulled up.) Do we need to support any compilers that don't support __VA_ARGS__ ? Even microsoft's compiler almost supports it. I don't know. --apb (Alan Barrett)
Re: CVS commit: src/sys/fs/puffs
On Sun, 05 Oct 2014, Alistair Crooks wrote: On Sun, Oct 05, 2014 at 02:13:15PM +, Alan Barrett wrote: #ifdef PUFFSDEBUG extern int puffsdebug; /* puffs_subr.c */ -#define DPRINTF(x) if (puffsdebug > 0) printf x -#define DPRINTF_VERBOSE(x) if (puffsdebug > 1) printf x +#define DPRINTF(x) do { \ + if (puffsdebug > 0) printf x; \ + while (/*CONSTCOND*/0) I think it'd be even more safe to close the block with a '}' before the while. Good idea. --apb (Alan Barrett)
Re: CVS commit: src/tools
On Tue, 30 Sep 2014, Joerg Sonnenberger wrote: Modified Files: src/tools: README Log Message: Say that tools should use C89, not C99; Given the state of the toolchain with effectively requiring C++ for both GCC 4.8 and Clang, does this really still make sense? The only relevant semi-modern toolchain I can think of without C99 support is MSVC and we don't support that anyway. Perhaps it does make sense to allow C99 in tools, but I think we have tried to avoid it until now. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/stdio
On Mon, 29 Sep 2014, Martin Husemann wrote: On Mon, Sep 29, 2014 at 08:29:10PM +0200, Steffen Nurpmeso wrote: (And also i would place a link to the current pull-ups (to the wiki) on www.netbsd.org/developers/releng/pullups.html, since Google shows the latter first, yet that is astonishing empty.) Can you say that again, please? I think he means: Please add links from <http://www.netbsd.org/developers/releng/pullups.html> to <https://releng.netbsd.org/index-7.html>, <https://releng.netbsd.org/index-6.html>, and <https://releng.netbsd.org/index-5.html>. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/gen
On Fri, 26 Sep 2014, Roy Marples wrote: Log Message: Remove \$ as a hidden marker as vis(3) wasn't setting it and it clobbered VIS_SHELL | VIS_CSTYLE. This is wrong. "vis -l" outputs \$, and with this change, unvis won't correctly handle it. unvis is not intended to reverse shell-style escapes. You can use the shell's eval command for that. Doesn't eval kind of defeat the purpose of shell sanitisation which VIS_SHELL is supposed to achieve? I can always add $ to "the don't encode this" list for VIS_CSTYLE. Yes, eval should be avoided if the input in untrusted. If unvis needs to handle both meanings of \$ (end of line for output from "vis -l", or '$' for output from the new shell escaping variant of vis) then it will need a flag to distinguish the cases. Or vis can be changed to use \044 instead of \$ in the shell escaping case, which I guess is what you meant by the "don't encode this" list. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/gen
On Fri, 26 Sep 2014, Roy Marples wrote: Modified Files: src/lib/libc/gen: unvis.c Log Message: Remove \$ as a hidden marker as vis(3) wasn't setting it and it clobbered VIS_SHELL | VIS_CSTYLE. This is wrong. "vis -l" outputs \$, and with this change, unvis won't correctly handle it. unvis is not intended to reverse shell-style escapes. You can use the shell's eval command for that. --apb (Alan Barrett)
Re: CVS commit: src/usr.sbin/sysinst
On Fri, 12 Sep 2014, Roy Marples wrote: Modified Files: src/usr.sbin/sysinst: net.c + /* +* Start dhcpcd quietly and in master mode, but restrict +* it to our interface +*/ + add_rc_conf("dhcpcd=YES\n"); + add_rc_conf("dhcpcd_flags=\"-qM %s\"\n", net_dev); This setting of dhcpcd_flags in /etc/rc.conf will restrict dhcpcd to only one interface. That might be what people want during the installation process, but it's not always what they want during future operations. For example, if I install on a laptop over a wired ethernet interface, I nevertheless want dhcpcd to work on the wifi interface later. I suspect that a more appropriate default would be for dhcpcd to run on all appropriate interfaces, unless the user deliberately restricts it. If you do want to restrict it by default, then please also write a comment to rc.conf to explain the consequences of this setting. In a hypothetical future where sysinst has a simple mode and an expert mode, I think simple mode should just let dhcpcd run on all interfaces, and expert mode should let the user choose a set of interfaces. Also, should we add -M to dhcpcd_flags in etc/defaults/rc.conf? Users typically customise those variables by copying the default settings from /etc/defaults/rc.conf to /etc/rc.conf and then adding additional options, so it's helpful if -M is already there before the user adds their own restricted list of interface names. --apb (Alan Barrett)
Re: CVS commit: src/distrib
On Fri, 12 Sep 2014, Roy Marples wrote: Modified Files: src/distrib/evbsh3/rom/ramdiskcommon: ramdiskbin.conf src/distrib/notes/common: main src/distrib/sets/lists/debug: mi Log Message: Remove more rtsol references. In distrib/notes, please don't just remove rtsol from the section about things that may be removed in the future. Please also add rtsol to the section about things that have been removed. --apb (Alan Barrett)
Re: CVS commit: src
On Thu, 11 Sep 2014, Roy Marples wrote: Log Message: Remove rtsol(8) and rtsold(8) as their functionality is in dhcpcd(8). Remove rtsol(8) from rc.d/network. Add -w seconds command to ifconfig to wait for N seconds for until DAD has finished on all addresses. Use ifconfig -w in rc.d/network instead of a forced sleep. This should have been at least three separate commits, more likely four. 1. add -w option to ifconfig; 2. use ifconfig -w option in rc.d/network; 3. stop using rtsol in rc.d/network; 4. remove rtsol and rtsold. With it all as one commit, it's unnecessarily difficult to pull up the "ifconfig -w" change and the rc.d/network changes to other branches. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/flock
On Mon, 18 Aug 2014, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Mon Aug 18 09:14:03 UTC 2014 Modified Files: src/usr.bin/flock: flock.c Log Message: make this behave like linux. Please describe the actual change, which seems to have something to do with changing defaults. Also, it probably needs a man page update. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/time
On Sat, 16 Aug 2014, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sat Aug 16 10:38:43 UTC 2014 Modified Files: src/lib/libc/time: zic.c Log Message: gcc on the sparc needs help with variables it thinks are unitialized, but are not. INITIALIZE(ktime); + ktime = 0; /* XXX: gcc */ The #define GNUC_or_lint and #ifdef GNUC_or_lint in time/private.h should have made it so INITIALIZE(ktime) was enough. --apb (Alan Barrett)
Re: CVS commit: src/sys/fs/ptyfs
On Wed, 13 Aug 2014, Juergen Hannken-Illjes wrote: Modified Files: src/sys/fs/ptyfs: ptyfs.h ptyfs_subr.c ptyfs_vfsops.c ptyfs_vnops.c Log Message: - Add a map of active controlling ptys per mount and no longer abuse the vnode lifecycle. - No longer set "recycle" on VOP_INACTIVE(). - Make ptyfs_used_get() private to ptyfs_subr.c - Stop copying device attributes from traditional ptys on first allocation. - Remove unneeded argument "lwp" from ptyfs_allocvp() and ptyfs_free_get(). Please could you update the mount_ptyfs(8) man page to explain the new behaviour. --apb (Alan Barrett)
Re: CVS commit: src
On Fri, 08 Aug 2014, John Nemeth wrote: } +++ src/distrib/sets/lists/base/miFri Aug 8 19:38:47 2014 } @@ -1,4 +1,4 @@ } -# $NetBSD: mi,v 1.1084 2014/08/08 19:34:35 apb Exp $ } +# $NetBSD: mi,v 1.1085 2014/08/08 19:38:47 apb Exp $ } # } # Note: Don't delete entries from here - mark them as "obsolete" instead, } #unless otherwise stated below. ^ Heh, thanks for the reminder. I'll fix it. --apb (Alan Barrett)
Re: CVS commit: src/sys/external/bsd/drm2/include/linux
On Sat, 26 Jul 2014, Taylor R Campbell wrote: Make Linux usecs_to_jiffies work with hz up to 2000. usecs_to_jiffies(unsigned int usec) { - return mstohz((usec + (1000 / hz) - 1) / (1000 / hz)); + if (hz <= 100) + return mstohz(roundup(usec, (1000 / hz))); + + /* +* Avoid integer overflow on 32-bit platforms. The cutoff is +* kinda arbitrary; for hz <= 2000, 0x20 is safe, but both +* values could wiggle around a little. +*/ + KASSERT(hz <= 2000); + if (usec <= 0x20) + return ((usec * hz) / 100); + else + return ((usec / 100) * hz); } You could just populate a struct timeval, and call tvtohz(9). What is the desired behaviour for small inputs? The code quoted above appears to round up when hz <= 100 (and to return 1 for very small but non-zero inputs) but to round down (and to return 0 for very small inputs) when hz > 100. Once the desired behaviour is clarified, you could implement it by making adjustments to the value before or after calling tvtohz. --apb (Alan Barrett)
Re: CVS commit: src/sys/dev/ata
On Fri, 25 Jul 2014, David A. Holland wrote: Modified Files: src/sys/dev/ata: wd.c Log Message: Drop the old discard/trim ioctls from wd.c. - case DIOCGDISCARDPARAMS: { - case DIOCDISCARD: { These never appeared in a release, so I suppose there's no need to implement these ioctls in any compat code. --apb (Alan Barrett)
Re: CVS commit: src
On Sat, 19 Jul 2014, Lourival Vieira Neto wrote: lua: updated from 5.1 to 5.3 work3 Is this a well-planned and well-tested update, or does it feel "rushed" to get it done before the netbsd-7 branch? I think this was as planned and as tested as Lua 5.1 import. However, maybe, I'm missing something. I don't see any recent discussion in public mailing lists. It was discussed on tech-kern [1] and also on icb/irc. [1] http://mail-index.netbsd.org/tech-kern/2014/07/12/msg017341.html Thanks. I had forgotten about that discussion. Two things that bothered me were that the version appears to be a pre-release, not the official Lua 5.3 release, and that the import is being done a very short time before the netbsd-7 branch is planned to be created. What I see in the brief discussion in tech-kern is reassuring, so I have no objection to this update. Also, if this doesn't get reverted for being rushed, then please import it on a vendor branch as per the usual practice. Did you mean moving it from -current to a specific branch? No, I mean using "cvs import" the way we usually do it. I'll explain in more detail in a private message. --apb (Alan Barrett)
Re: CVS commit: src
On Sat, 19 Jul 2014, Lourival Pereira Vieira Neto wrote: lua: updated from 5.1 to 5.3 work3 Is this a well-planned and well-tested update, or does it feel "rushed" to get it done before the netbsd-7 branch? I don't see any recent discussion in public mailing lists. Also, if this doesn't get reverted for being rushed, then please import it on a vendor branch as per the usual practice. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc
On Tue, 24 Jun 2014, David Holland wrote: On Mon, Jun 23, 2014 at 07:46:15PM +, Taylor R Campbell wrote: Read from /dev/urandom. ...ugh. Can we provide a wrapper around this for transparent casual use? (Even if it's in libutil and marked not for general consumption?) Yes please. Note that we already have cprng_{fast,strong}{,32,64} functions in the kernel. Or it may be worthwhile to mostly keep the way arc4random(3) works but replace the PRNG, as in the first reimplementation of arc4random(3) above, but rename it. yes please Yes, I think that's what we are getting at. Keep the way the interface works, to make it easy for callers to adapt to the new way, reimplement the internals, and rename to something that does not embed an algorithm name in the function name. With either of the later two cases, perhaps we ought to just coopt random(3) for the purpose. no please (random(3) is not expected to be cryptographically strong, so code that assumes it is becomes unportable in a subtle and dangerous way) Also I think there's code out there that saves and restores the random(3) state and expects to get repeatable results. Yes, the random(3) functions need to be able to provide repeatable results, and state save/restore functionality. I am not sure about repeatability requirements across OS upgrades. I think it may be acceptable for different versions of the random(3) implementation (say before and after an OS upgrade) to produce different output streams from the same initial seed or state. --apb (Alan Barrett)
Re: CVS commit: src/lib/librumpuser
On Mon, 16 Jun 2014, Alexander Nasonov wrote: Log Message: Add __RCSID. +#if !defined(lint) +__RCSID("$NetBSD: rumpuser_bio.c,v 1.8 2014/06/16 21:07:28 alnsn Exp $"); +#endif /* !lint */ Some historical uses of __RCSID have an unnecessary #if/#endif wrapper around them, but for new uses, please just write __RCSID(...); without any #if/#endif wrapper. --apb (Alan Barrett)
Re: CVS commit: src
On Tue, 10 Jun 2014, Joerg Sonnenberger wrote: Log Message: Introduce new sysctls for obtaining interface-specific addresses: - net.sdl for the active link-layer adddress (the MAC) - net.ether.multicast for the Ethernet multicast addresses - net.inet6.multicast for the IPv6 multicast groups - net.inet6.multicast_kludge for temporarily removed multicast groups Please document these in sysctl(7). --apb (Alan Barrett)
Re: CVS commit: src/distrib/sets
On Sat, 24 May 2014, Masao Uebayashi wrote: Log Message: Unbreak syspkg by escaping '[' by vis(1) to match the new mtree(8) format. I'd prefer to escape using TOOL_SED so we don't need to add a new TOOL_VIS. --apb (Alan Barrett)
Re: CVS commit: src/share/zoneinfo
On Sat, 17 May 2014, Christos Zoulas wrote: Add tzdata2netbsd, a script to help import new versions of tzdata. Why all the || exit $? instead of set -e set -e would probably be better. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/xlint/lint1
On Fri, 18 Apr 2014, matthew green wrote: don't include fmemopen in tools builds. Since tools does not define _NETBSD_SOURCE, we don't get the fmemopen prototype hmm is that because we define _SOMEOTHER_SOURCE? _NETBSD_SOURCE should be defined normally, unless you start to ask for restricted namespaces... src/tools/compat/compat_defs.h goes to a lot of trouble to ensure that _NETBSD_SOURCE does not get defined. I don't understand why that is done. --apb (Alan Barrett)
Re: CVS commit: src/common/lib/libc/string
On Mon, 14 Apr 2014, Joerg Sonnenberger wrote: Modified Files: src/common/lib/libc/string: bcopy.c Log Message: Using bcopy/memcpy with NULL arguments is valid as long as the size is also 0. No, it's undefined behaviour. C99 section 7.21.1: Unless explicitly stated otherwise in the description of a particular function in this subclause, pointer arguments on such a call shall still have valid values, as described in 7.1.4. and 7.1.4 says: If an argument to a function has an invalid value (such as ... a null pointer ...) ..., the behavior is undefined. and 7.21.2.1 "The memcpy function" does not give any explicit permission for use of null pointers. I don't object if the implementation wants to allow null pointers with zero size as an extension, but it should be clear that this is non-standard. --apb (Alan Barrett)
Re: CVS commit: [tls-earlyentropy] src/distrib/utils/sysinst
On Wed, 09 Apr 2014, Thor Lancelot Simon wrote: Modified Files: src/distrib/utils/sysinst [tls-earlyentropy]: util.c Log Message: Try to persistently gather some entropy at install time, to give the fresh system a better chance of not doing awful things like generating guessable SSH host keys. Handles both systems with /var on / and /var on its own filesystem. Tries to preserve old saved entropy when upgrading. I see that you chose to use /etc/entropy-file when /var/db/entropy-file is not on the root file system. Some other locations that I would consider include: /stand/ -- the entropy file may be used by the boot loader before a kernel is running, so that fits, but it's not a "program", so that doesn't fit the description in hier(7). /libdata/ -- the entropy file is a non-executable file that is required at boot time, which seems to match the description in hier(7) perfectly. --apb (Alan Barrett)
Re: CVS commit: src/sys/sys
On Fri, 14 Mar 2014, Joerg Sonnenberger wrote: Modified Files: src/sys/sys: cdefs.h Log Message: For compilers without __COUNTER__, make the ctassert name contain __INCLUDE_LEVEL__ ## _ ## __LINE__. It's not perfect, but at least it's better than just __LINE__ since it avoids collisions between .c's and .h's. This is wrong, __INCLUDE_LEVEL__ does not exist unconditionally. You could define a special token in each include file, and then use that token if it is defined. You could also use __INCLUDE_LEVEL__ only if it's defined. Something like this (untested): /* in foo/bar.h: */ #ifndef _FOO_BAR_H #define _FOO_BAR_H #undef __HEADER_NAME_TOKEN #define __HEADER_NAME_TOKEN _FOO_BAR_H /* insert real content here */ #endif /* _FOO_BAR_H */ /* in sys/cdefs.h: */ #define __CTASSERT0(x, y, z1, z2, z3)__CTASSERT1(x, y, z1, z2, z3) #define __CTASSERT1(x, y, z) \ typedef char y ## z1 ## z2 ## z3[/*CONSTCOND*/(x) ? 1 : -1] __unused #if defined(__COUNTER__) # define __CTASSERT(x) __CTASSERT0(x, __ctassert, __COUNTER__,,) #else # if defined(__INCLUDE_LEVEL__) # define __tmp_z1 __INCLUDE_LEVEL__ # else # define __tmp_z1 /* Empty */ # endif # if defined(__HEADER_NAME_TOKEN) # define __tmp_z2 __HEADER_NAME_TOKEN # else # define __tmp_z2 /* Empty */ # endif # define __CTASSERT(x) _CTASSERT0(x, __ctassert, __tmp_z1, __tmp_z2, __LINE__) # undef __tmp_z1 # undef __tmp_z2 #endif --apb (Alan Barrett)
Re: CVS commit: src/sys/kern
On Wed, 05 Mar 2014, Andreas Gustafsson wrote: Changing the types of existing sysctl variables breaks both source and binary compatibility and should not be done lightly; Changing the types without sufficient care can break source and binary compatibility. With sufficient care, compatibility can be maintained, and I thought that dsl had attempted to do that. However, I think that a change like that should have been discussed first. that's why we have both both hw.physmem and hw.physmem64, for example. I think that it was a mistake (made several years ago) to introduce hw.physmem64, and that hw.physmem should have been extended to allow larger values, in a backward compatible way, very much like whay dsl has attempted to do now. Some details might be wrong, and it should have been discussed first, but the principle of returning what the caller expects seems reasonable to me. I believe the harm caused by the incompatible type change far outweighs the cost of a few added lines of code, and would like the original types to be restored. I agree that an incompatible type change would be harmful. If that problem exists, then it could be fixed either by changing the types back, or by fixing the compatibility. Without knowing the reason for the type change, I don't know which of those options I prefer in the long term. For the short term, read on. 2. I also object to the change of kern_syctl.c 1.247. This change attempts to work around the problems caused by the changes to the variable types by making sysctl() return different types depending on the value of the *oldlenp argument. IMO, this is a bad idea. The *oldlenp argument does *not* specify the size of the data type expected by the caller, but rather the size of a buffer. The sysctl() API allows the caller to pass a buffer larger than the variable being read, and conversely, guarantees that passing a buffer that is too small results in ENOMEM. Both of these aspects of the API are now broken: reading a 4-byte CTLTYPE_INT variable now works for any buffer size >= 4 *except* 8, and attempting to read an 8-byte CTLTYPE_QUAD variable into a buffer of less than 8 bytes is now guaranteed to yield ENOMEM *except* if the buffer size happens to be 4. IMO, this behavior violates both the letter of the sysctl() man page and the principle of least astonishment. Also, the work-around is ineffective in the case of a caller that allocates the buffer dynamically using the size given by an initial sysctl() call with oldp = NULL. Yes, I think you are right about the details, and we should probably revert the change, at least until a design is discussed that meets all reasonable requirements for compatibility. --apb (Alan Barrett)
Re: CVS commit: src/sys/kern
On Thu, 27 Feb 2014, David Laight wrote: Modified Files: src/sys/kern: kern_sysctl.c + case CTLTYPE_INT: + /* Allow for 64bit read of 32bit value */ + if (*oldlenp == sizeof (uint64_t)) { + qval = *(int *)d; + d_out = &qval; + sz = sizeof (uint64_t); + } + break; + case CTLTYPE_QUAD: + /* Allow for 32bit read of 64bit value */ + if (*oldlenp == sizeof (int)) { + qval = *(uint64_t *)d; + ival = qval < 0x1 ? qval : 0x; + d_out = &ival; + sz = sizeof (int); + } + break; This will fail if int is not a 32-bit type, because it uses hardcoded masks instead of adapting to the actual size. --apb (Alan Barrett)
Re: CVS commit: src/sys/dev/pci
On Thu, 27 Feb 2014, Joerg Sonnenberger wrote: Modified Files: src/sys/dev/pci: if_ti.c Log Message: Remove impossible checks. If new bugs are introduced in the future, then these tests might no longer be impossible. So I'd prefer to change them to KASSERT or KDASSERT rather than deleting them. --apb (Alan Barrett)
Re: CVS commit: src/sys/uvm
On Wed, 26 Feb 2014, Matt Thomas wrote: Modified Files: src/sys/uvm: uvm_meter.c uvm_param.h Log Message: Add vm.min_address and vm.max_address which return VM_MIN_ADDRESS and VM_MAXUSER_ADDRESS. Do these need to use the old style #define constants instead of the new style (since 2005) CTL_CREATE idiom? --apb (Alan Barrett)
Re: CVS commit: src/bin/ls
On Thu, 20 Feb 2014, Christos Zoulas wrote: Modified Files: src/bin/ls: ls.1 ls.c ls.h print.c Log Message: Add -O (only leaf files) and -P (print full path), from tls@ Do any other ls implementations have such options? I like the idea of being able to run "ls -ROP1 ." instead of "find . type f -print", but I'd like the options to be compatible with other operating systems. --apb (Alan Barrett)
Re: CVS import: src/external/bsd/atf/dist
On Tue, 11 Feb 2014, Julio Merino wrote: Log Message: Import atf-0.20: [...] * Removed the deprecated tools. This includes atf-config, atf-report, atf-run and atf-version. When were those tools deprecated (or, for how long have we had both the deprecated old tools and their new replacements), and what are the replacements? I see no mention of deprecation in the man page for a version of atf-run from November 2013. --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/arm/include
On Wed, 29 Jan 2014, Matt Thomas wrote: Modified Files: src/sys/arch/arm/include: int_fmtio.h int_mwgwtypes.h Log Message: Make {,u}int{8,16,32} be of type int. I think you mean: Make {,u}int_fast{8,16,32} be of type int. --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/i386/include
On Mon, 27 Jan 2014, Christos Zoulas wrote: Modified Files: src/sys/arch/i386/include: vmparam.h Log Message: Cut down MAXDSIZE from 3G to 2.5G otherwise bottomup allocation ends up supplying an out of bounds hint for sigcode (c001e000 > bf00). Makes a.out binaries work again. Will this make malloc fail 0.5GB earlier than before? The data size limits on i386 are already annoyingly small, and I would prefer not to make them smaller. Please could you find a way to penalise only a.out programs. --apb (Alan Barrett)
Re: CVS commit: src/distrib/sets
On Fri, 24 Jan 2014, Masao Uebayashi wrote: I agree that in an ideal reproducible world timestamp (== physical time and its order) has no value. But it is useful to detect "unnecessary rebuild" - reproducible but built repeatedly & unnecessarily. I see some value in it. You could extract that from METALOG. If something is built twice, it will have two entries in METALOG, but only one entry in METALOG.sanitised. Depending on the host platform, there might not be any timestamps in METALOG. On a NetBSD host, I see time= keywords in METALOG, but on a Linux host, I do not see them. --apb (Alan Barrett)
Re: CVS commit: src/share/mk
On Tue, 21 Jan 2014, Matt Thomas wrote: Module Name:src Committed By: matt Date: Tue Jan 21 16:40:24 UTC 2014 Modified Files: src/share/mk: bsd.own.mk Log Message: Make MKGCCCMDS default mirror MKGCC. (if MKGCC is no, MKGCCCMDS must be no). If that's true, then please adjust bsd.README to say so. Currently, they are documented as being independent (but there seem to be some accidental yes/no inversions in the documentation). --apb (Alan Barrett)
Re: CVS commit: src/share/mk
On Wed, 22 Jan 2014, Christos Zoulas wrote: On Jan 22, 7:29am, m...@3am-software.com (Matt Thomas) wrote: -- Subject: Re: CVS commit: src/share/mk | I always wondered why we don't use ln -sf=20 | and avoid the race. That does not work because if the destnation is a directory it will try to link in the destination directory... (I tried). This is why I suggested that it needs to be done differently. In the common case, all this linking is happening in an OBJDIR which either started off empty, or started off with the correct symlinks. It might be possible to detect this common case and arrange for it to be race-free. I don't think "ln -sf" is so bad. The first time, you get the desired symlink: /dirname/linkname -> destination then the second time, if you lose the race, you get a useless and mostly harmless extra link: /dirname/destination/destination -> destination My tests suggest that the third time, and every other time you run "ln -sf destination /dirname/linkname", nothing really changes. /dirname/destination/destination might be deleted and re-created, but (at least on NetBSD and Linux) nothing worse happens. If the extra link is offensive, then just delete it, making the entire sequence like this: # remove old symlink rm -f /dirname/linkname # create desired new symlink, # or create bogus extra link (if we lose the race) ln -sf destination /dirname/linkname # remove bogus extra link (if it exists) rm -f /dirname/destination/destination "ln -sfh" is not portable, so we can't use that. --apb (Alan Barrett)
Re: CVS commit: src/external/bsd/file/dist/src
On Thu, 16 Jan 2014, Joerg Sonnenberger wrote: Module Name:src Committed By: joerg Date: Thu Jan 16 23:36:52 UTC 2014 Modified Files: src/external/bsd/file/dist/src: file.c Log Message: Only use __dead if it exists. I think the code would be easier to read with #ifndef __dead #define __dead /* nothing */ #endif --apb (Alan Barrett)
ntp.conf
On Mon, 06 Jan 2014, Alan Barrett wrote: On Mon, 06 Jan 2014, Erik Fair wrote: Unless I misunderstand NTP configuration semantics, your additional "restrict" statements for the NTP pool names will do the wrong thing, in that each reference to a given netbsd.pool.ntp.org name returns multiple IP addresses, in apparently random order, i.e. an attempt to guarantee no two queries return the same data. Ergo, those restrict statements will most probably not end up with the same IP address as their preceding "server" statements, as was presumably your intent. Yes, you are correct. What should we do? If you have "restrict default nopeer noquery" (the uncommented line in my commit), then time service will still work, but the configured servers will be denied query permission. If you use "restrict default ignore", then time service does not work. I propose to remove the commented-out "restrict default ignore", remove the various "restrict *.netbsd.pool.ntp.org" lines, and remove all mention of special "restrict" lines for each peer or server, and change a few comments. That will leave the restrict-related part of the default configuration like this: [[[ # Access control restrictions. # See /usr/share/doc/html/ntp/accopt.html for syntax. # See <http://support.ntp.org/bin/view/Support/AccessRestrictions> for advice. # Last match wins. # # Some of the more common keywords are: # ignore Deny packets of all kinds. # kod Send "kiss-o'-death" packets if clients exceed rate # limits. # nomodifyDeny attempts to modify the state of the server via # ntpq or ntpdc queries. # noquery Deny all ntpq and ntpdc queries. Does not affect time # synchronisation. # nopeer Prevent establishing new peer associations. # Does not affect peers configured using "peer" lines. # Does not affect client/server time synchronisation. # noserve Deny all time synchronisation. Does not affect ntpq or # ntpdc queries. # notrap Deny the trap subset of the ntpdc control message protocol. # notrust Deny packets that are not cryptographically authenticated. # # By default, allow client/server time exchange without prior # arrangement, but deny configuration changes, queries, and peer # associations that were not explicitly configured. # restrict default kod nopeer noquery # Fewer restrictions for the local subnet. # (Uncomment and adjust as appropriate.) # #restrict 192.0.2.0 mask 255.255.255.0 kod nomodify notrap nopeer #restrict 2001:db8:: mask ::: kod nomodify notrap nopeer # No restrictions for localhost. # restrict 127.0.0.1 restrict ::1 ]]] Does that sound OK? --apb (Alan Barrett)
Re: CVS commit: src/etc
On Sat, 11 Jan 2014, Erik Fair wrote: On Jan 8, 2014, at 10:25 , Greg Troxel wrote: Why do you think the configured servers should be given query permission? Is that a sense of courtesy to the pool operators that they should be able to run "ntpdc -c monlist" and "ntpq -p" at machines that are syncing from them? Yes, that's polite. It is suggested by the NTP peering guidelines (and I know "server" and "peer" are different). I am sure I have heard this advice in the past, but I can't find a reference to it. --apb (Alan Barrett)
use of bsd.sys.mk
On Fri, 10 Jan 2014, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Fri Jan 10 15:50:34 UTC 2014 Modified Files: src/external/mit/xorg/server/xorg-server/hw/xfree86/common: Makefile Log Message: don't include bsd.sys.mk bsd.sys.mk is needed to get the value of HOST_SH, which is used in a "!=" line. At present, Makefiles that use assorted HOST_* tools in "!=" lines usually include bsd.sys.mk to get the variable definitions sufficiently early. If you don't want them to do that, then we need to find a way that works. Perhaps a lot of the variable definitions from bsd.sys.mk can be moved to bsd.own.mk, or a file that's included by bsd.own.mk. Also, if you think that Makefiles should not include bsd.sys.mk, then please edit share/mk/bsd.README to document how things should be done. --apb (Alan Barrett)
Re: CVS commit: src/etc
On Mon, 06 Jan 2014, Erik Fair wrote: Unless I misunderstand NTP configuration semantics, your additional "restrict" statements for the NTP pool names will do the wrong thing, in that each reference to a given netbsd.pool.ntp.org name returns multiple IP addresses, in apparently random order, i.e. an attempt to guarantee no two queries return the same data. Ergo, those restrict statements will most probably not end up with the same IP address as their preceding "server" statements, as was presumably your intent. Yes, you are correct. What should we do? If you have "restrict default nopeer noquery" (the uncommented line in my commit), then time service will still work, but the configured servers will be denied query permission. If you use "restrict default ignore", then time service does not work. --apb (Alan Barrett)
Re: CVS commit: src
On Mon, 02 Dec 2013, Lourival Pereira Vieira Neto wrote: Module Name:src Committed By: lneto Date: Mon Dec 2 04:39:10 UTC 2013 Modified Files: src/lib/libc/stdlib: Makefile.inc src/sys/lib/libkern: Makefile.libkern libkern.h src/sys/modules/lua: Makefile luaconf.h Added Files: src/common/lib/libc/stdlib: strtoimax.c Removed Files: src/lib/libc/stdlib: strtoimax.c Log Message: changed lua_Number to int64_t This was actually two logical changes: * move strtoimax from the userland-only part of libc to the common part that's also usable by the kernel. * change lua_Number to int64_t for the kernel Lua implementation. I would have preferred to see two separate commits. Even with only one commit, the log message should have described both changes. --apb (Alan Barrett)
Re: CVS commit: src/sys
On Sat, 23 Nov 2013, Jeff Rizzo wrote: Log Message: Since mountlist is now a TAILQ, convert some missed usages so things build again. It would be better if the callers were rewritten to use the queue.h macros instead of direct access to the struct members. (Especially since the names of the struct members are not part of the documented API.) For example, this ... -for (mp = mountlist.cqh_first; mp != (void*)&mountlist; mp = nmp) { - nmp = mp->mnt_list.cqe_next; +for (mp = mountlist.tqh_first; mp != (void*)&mountlist; mp = nmp) { + nmp = mp->mnt_list.tqe_next; ... could be rewritten to use TAILQ_FOREACH. --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/i386/conf
On Fri, 22 Nov 2013, Jeff Rizzo wrote: Modified Files: src/sys/arch/i386/conf: ALL Log Message: Comment out npf for now, as we can't have both NPF and PF in the same kernel It used to be possible to have npf and pf in the same kernel. The kernel I am running now (built a few weeks ago) has both. I think it's important for users to be able to have both. If I want to migrate from pf to npf, then part of my testing would probably involve switching back and forth a few times to check that the npf rules do the same job as the pf rules, and I would want to be able to do that without booting a different kernel for every test. - rmind has said he'll address this eventually, OK. --apb (Alan Barrett)
Re: CVS commit: src/sys/compat/linux
On Mon, 18 Nov 2013, Chuck Silvers wrote: Module Name:src Committed By: chs Date: Mon Nov 18 01:32:22 UTC 2013 Modified Files: src/sys/compat/linux/arch/amd64: linux_exec.h linux_exec_machdep.c src/sys/compat/linux/common: linux_exec.h linux_exec_elf32.c Log Message: implement AT_RANDOM. random() returns a predictable value based on a seed. Given the supposed use of AT_RANDOM for security, I think that it would be more appropriate to use a stronger source, such as cprng_strong32(). --apb (Alan Barrett)
Re: CVS commit: src/sys/modules/lua
On Thu, 31 Oct 2013, Marc Balmer wrote: Modified Files: src/sys/modules/lua: Makefile Log Message: fix build on arm Please describe the actual change, not just the reason for it. Fopr example: Move "-include ${.CURDIR}/luaconf.h" from CPPFLAGS to CFLAGS. --apb (Alan Barrett)
Re: CVS commit: src
On Mon, 28 Oct 2013, Marc Balmer wrote: Modified Files: src/distrib/sets/lists/modules: mi src/sys/modules: Makefile Log Message: linke pmf(9l) to the build Messages like this would be more useful if they said "9lua" instyead of "9l". --apb (Alan Barrett)
Re: CVS commit: src/sys/sys
On Fri, 25 Oct 2013, Jukka Ruohonen wrote: src/sys/sys: cdefs.h Log Message: Add __diagused and __debugused. These are for marking variables that are used only in diagnotic or debug code, but unused when NDEBUG is defined, or DIAGNOSTIC is not defined, or DEBUG is not defined. I tried to collect some of these to attribute(3), which might be worth updating (or not). Thank you! I didn't know about attribute(3). --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/sparc
On Mon, 21 Oct 2013, matthew green wrote: > #define raise_ipi(cpi,lvl) do {\ >- volatile int x; \ >+ int x; \ >(cpi)->intreg_4m->pi_set = PINTR_SINTRLEV(lvl); \ >- x = (cpi)->intreg_4m->pi_pend;\ >+ x = (cpi)->intreg_4m->pi_pend; __USE(x); \ > } while (0) I think that the change from the use of volatile to the use of __USE() is a change from reliance on the C standard's guarantee that the memory location behind (cpi)->intreg_4m->pi_pend really will be accessed, to a reliance on what a particular compiler happens to do in this situation. the volatile applies to the access to pi_pend, which is marked a volatile (via the whole intreg_4m pointer.) Oh, if cpi, or intreg_4m, or pi_pend, is volatile, then it's fine, and that would be the best way to deal with the issue. In such a case, there's no need to use x at all. i'm not sure that "volatile int x;" actually means anything useful. it's pointer accesses that matter. i could be wrong; volatile is a pretty tricky area. Yes, the pointer access is the thing that really needs to be volatile, but I previously thought that it was not, and then I thought (incorrectly, I now believe) that making x volatile would be sufficient. certainly, the generated code is fine (and in most cases, identical). My concern was with other compilers in the future. If the pointer access is volatile, then I think it's fine. --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/sparc
On Sat, 19 Oct 2013, matthew green wrote: Module Name:src Committed By: mrg Date: Sat Oct 19 19:40:23 UTC 2013 Modified Files: src/sys/arch/sparc/dev: if_ie_obio.c kd.c tctrl.c ts102.c src/sys/arch/sparc/include: pmap.h src/sys/arch/sparc/sparc: cpuvar.h memecc.c timer.c Log Message: - remove unused but set variables. - use __USE() where necessary. - remove useless 'volatile' markers I am not sure that those volatile markers are useless. For example: #define raise_ipi(cpi,lvl) do {\ - volatile int x; \ + int x; \ (cpi)->intreg_4m->pi_set = PINTR_SINTRLEV(lvl); \ - x = (cpi)->intreg_4m->pi_pend;\ + x = (cpi)->intreg_4m->pi_pend; __USE(x); \ } while (0) I think that the change from the use of volatile to the use of __USE() is a change from reliance on the C standard's guarantee that the memory location behind (cpi)->intreg_4m->pi_pend really will be accessed, to a reliance on what a particular compiler happens to do in this situation. --apb (Alan Barrett)
Re: CVS commit: src/sys/modules/lua
On Fri, 18 Oct 2013, Marc Balmer wrote: Did you get core approval for this? The public discussion is still ongoing, and still lacking in evidence, and there has been no public statement by core as far as I am aware, nor did any members of core I asked have any recollection of approving this. Please don't steamroll over public discussions like this. Just for the record, yes, I got core's approval. Could you tell us approximately when you got that approval? It wasn't in the past two years while I have been a member of core. --apb (Alan Barrett)
Re: CVS commit: xsrc/external/mit/xdm/dist/xdm
On Fri, 18 Oct 2013, Jukka Ruohonen wrote: Print time_t values by casting to intmax_t and using "%ji" format. Wouldn't the "right" way be PRIdMAX from inttypes(3)? Anyways, for curiosity: is there actually a portable way across operating systems to deal with this? The "j" modifier is defined in the C99 standard, and that's good enough for me. PRIdMAX would also work, but is more verbose. I suppose some operating systems might exist where "%jd" doesn't work but "%"PRIdMAX does work, and if this was the xorg project instead of the NetBSD project then I might care about that. --apb (Alan Barrett)
Re: CVS commit: xsrc/external/mit/xdm/dist/xdm
On Fri, 18 Oct 2013, Christos Zoulas wrote: The result from strlen() has type size_t, so print it with "%zd" format. %zu is correct. Fixed, thanks.
Re: CVS commit: src/crypto/external/bsd/openssh/dist
On Sun, 06 Oct 2013, Jean-Yves Migeon wrote: Modified Files: src/crypto/external/bsd/openssh/dist: ssh_config Log Message: Enable VerifyHostKeyDNS (SSHFP records verification) from DNS for hosts under NetBSD.org domain. Thank you. I think this is an improvement. Notified on netbsd-users@, no objection after a week -- committed. Please discuss such things in the relevant tech-* list (tech-net or tech-userlevel in this case, I suppose). +# NetBSD.org DNS provides SSHFP records - use them when possible +Host *.netbsd.org *.NetBSD.org +VerifyHostKeyDNS ask I have been running similar configuration for some time, but with with "VerifyHostKeyDNS yes" (not "ask"), and I have had no problems. The difference between "yes" and "ask" arises only when the ssh client can be sure that the DNS answer was secured by DNSSEC; in such a case, "yes" means accept the result silently, while "ask" means ask the user (the first time). If the DNS answer was not secured by DNSSEC, then both "yes" and "ask" end up asking the user. By the way, I think that's a bug in ssh that the Host patterns are case sensitive. --apb (Alan Barrett)
Re: CVS import: src/external/gpl3/binutils/dist
On Sun, 29 Sep 2013, Christos Zoulas wrote: | > Log Message: | > from ftp.gnu.org | > | > Status: | > | > Vendor Tag: FSF | > Release Tags:binutils-2-23-2 | | thanks for working on this. please include a real commit log | next time. this one doesn't even mention what package or | version, let alone a simple list of changes since our last | update. So next time: I mention that it is binutils.2.23.2 like it says above, and I copy the ChangeLog? Yes, please mention that it is binutils-2.32.2. The existence of the "binutils-2-23-2" tag, or the fact that it corresponds to this import, will not be obvious to future users of "cvs log". If the ChangeLog is small, then pasting it into the commit log is useful. If the ChangeLog is large, then please extract only the highlights. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/make
On Mon, 02 Sep 2013, Simon J. Gerraty wrote: Modified Files: src/usr.bin/make: compat.c Log Message: Do not apply shellErrFlag unless errCheck is true. To generate a diff of this commit: cvs rdiff -u -r1.92 -r1.93 src/usr.bin/make/compat.c Will this fix PR 45356? --apb (Alan Barrett)
Re: CVS commit: src/bin/csh
On Tue, 06 Aug 2013, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Tue Aug 6 05:42:43 UTC 2013 Modified Files: src/bin/csh: lex.c Log Message: CID 1060854: Wrong sizeof argument (SIZEOF_MISMATCH) Does everybody know what "CID" means? I'd be inclined to say "Coverity CID " instead of just "CID " in these log messages. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/time
On Thu, 01 Aug 2013, David Holland wrote: (I thought time_t was required to be an integer type, but I suppose there's some legacy platform where it isn't.) C99 requires time_t to be an arithmetic type. C99 section 7.23.1 point 3 "The types declared are ... clock_t and time_t which are arithmetic types capable of representing times; ..." POSIX adds that time_t must be an integer type. <http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html> "clock_t shall be an integer or real-floating type. [CX] [Option Start] time_t shall be an integer type. [Option End]" (The "[CX][Option Start]...[Option End]" just means "Extension to he ISO C standard".) The tzcode implementation goes to some trouble to be portable to environments with integer or floating point types for time_t. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/time
On Wed, 31 Jul 2013, David Holland wrote: On Tue, Jul 30, 2013 at 03:30:37PM +, Joerg Sonnenberger wrote: > Modified Files: >src/lib/libc/time: localtime.c > > Log Message: > Don't depend on implicit rounding from non-integral float constant. what on earth is that code trying to do? If time_t is a floating point type, then round to the nearest second. If time_t is an integral type, then do nothing. There really should be a comment to explain that. --apb (Alan Barrett)
Re: CVS commit: [riastradh-drm2] src/sys/external/bsd/drm2/include/linux
On Wed, 24 Jul 2013, Taylor R Campbell wrote: Modified Files: src/sys/external/bsd/drm2/include/linux [riastradh-drm2]: kernel.h Log Message: Implement bogus max_t in . +/* XXX These will multiply evaluate their arguments. */ +#definemax_t(T, X, Y) MAX(X, Y) #define min_t(T, X, Y) MIN(X, Y) If we are willing to use the non-standard ({ ... }) syntax, then these can be written to evaluate their arguments only once, like this: #define max_t(T, X, Y) ({ T _x = (X), _y = (Y); (_x > _y ? _x : _y); }) #define min_t(T, X, Y) ({ T _x = (X), _y = (Y); (_x < _y ? _x : _y); }) --apb (Alan Barrett)
Re: CVS commit: [riastradh-drm2] src/sys/external/bsd/drm2/dist/drm/i915
On Wed, 24 Jul 2013, Taylor R Campbell wrote: Modified Files: src/sys/external/bsd/drm2/dist/drm/i915 [riastradh-drm2]: intel_dp.c Log Message: Avoid {0} struct initializer in intel_dp_init_connector. The usual way we deal with this in NetBSD is to write typename varname = {.membername = 0}; where membername is any valid member of the struct. I wish gcc -Wmissing-field-initializers would recognise that {0} is a special case that does nto deserve a warning, so we could write the natural and well-defined code typename varname = {0}; --apb (Alan Barrett)
Re: CVS commit: [riastradh-drm2] src/sys/external/bsd/drm2/include/linux
On Wed, 24 Jul 2013, Taylor R Campbell wrote: Module Name:src Committed By: riastradh Date: Wed Jul 24 01:54:19 UTC 2013 Modified Files: src/sys/external/bsd/drm2/include/linux [riastradh-drm2]: types.h Log Message: Define cycles_t to be unsigned long long for now in . Could we use a fixed-size type? Perhaps uint64_t? If this type is never exposed to userland then it doesn't matter, but if it is exposed to userland then a fixed-size type makes it easier to provide compatibility between 32-bit and 64-bit versions of related platforms. i386 and amd64 both have 64-bit long long, so it doesn't affect them, but other pairs of related platforms (perhaps in the future) might be different. --apb (Alan Barrett)
Re: CVS commit: src/bin/hostname
On Fri, 19 Jul 2013, Roy Marples wrote: Module Name:src Committed By: roy Date: Fri Jul 19 10:34:51 UTC 2013 Modified Files: src/bin/hostname: hostname.1 hostname.c If we want these changes (which is by no means clear -- I can't find any discussion of these new options, what they are good for, or why we want them), then we should at least document them better. +.It Fl A +Display the FQDN of each address on all interfaces. +.It Fl a +Display alias name(s) of the host. +.It Fl d +Display the DNS domain. +.It Fl f +Display the FQDN for the hostname. +.It Fl I +Display each IP address on all interfaces. +.It Fl i +Display the IP address(es) for the hostname. .It Fl s -Trims off any domain information from the printed -name. +Display the short hostname. The man page should define all these terms, and explain how they are related to each other. --apb (Alan Barrett)
Re: CVS commit: src/bin/hostname
On Fri, 19 Jul 2013, Erik Fair wrote: Modified Files: src/bin/hostname: hostname.1 hostname.c Log Message: Add the following options -A Display the FQDN of each address on all interfaces. -a Display alias name(s) of the host. -d Display the DNS domain. -f Display the FQDN for the hostname. -I Display each IP address on all interfaces. -i Display the IP address(es) for the hostname. Not to go all Rob Pike on you (cf. "cat -v considered harmful"), but what the heck is all this for? And also: Where was this change discussed? --apb (Alan Barrett)
Re: CVS commit: src/share/zoneinfo
On Sun, 07 Jul 2013, Erik Fair wrote: Pullup request to netbsd-6 and netbsd-5? Yes, I always request such pullups. I like to wait a day or two, but it's occasionally taken me much longer than that. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/string
On Mon, 24 Jun 2013, Taylor R Campbell wrote: Discuss on tech-userlevel? Yes please. --apb (Alan Barrett)
Re: CVS commit: src
On Sun, 23 Jun 2013, Taylor R Campbell wrote: From: chris...@astron.com (Christos Zoulas) We should not be creating new interfaces based on obsolete ones. I'd rather we have consttime_memcmp() and explicit_memset(). I don't care if we rename them to consttime_memeq and explicit_memzero instead, but these are absolutely the operations that real crypto applications need. consttime_memcmp and explicit_memset are not, and are needlessly complicated. I see the point about avoiding names that resemble the deprecated functions bcmp() and bzero(). I also agree with the point that the memcmp() interface is not what is wanted here. I don't care so much about the difference between the bzero() and memset() interfaces. Because the implementation of consttime_memcmp() would be complex, I'd object to adding it unless there's a caller that needs the three-state return value. I think that the names explicit_memzero() and consttime_memeq() are fine, but I'd expect consttime_memeq() have the opposite polarity from consttime_bcmp(), because it should be answering the question "are they equal?" (1 for true), not "in what order do they compare for sorting?" (0 for equal). By the way, I have an implementation of consttime_memcmp() at <ftp://ftp.netbsd.org/pub/NetBSD/misc/apb/consttime_memcmp.c>. --apb (Alan Barrett)
Re: CVS commit: xsrc/external/mit/xauth/dist
On Mon, 03 Jun 2013, Christos Zoulas wrote: | > Well, there is an advantage that the FreeBSD one has over ours. It can be | > used in c++ with -Wold-style-cast, if defined as: | > | > #define __DECONST(t, a) const_cast(a) | | That, and why is it cast to an unsigned long and not uintptr_t? So that it does not bring in another header to define uintptr_t. unsigned long might not be long enough. I'd rather bring in another header than have code that might do the wrong thing on some future platform. We should probably have a header that defines "__" versions of some useful types without polluting the namespace, and then use __intptr_t. --apb (Alan Barrett)
Re: CVS commit: src
On Wed, 29 May 2013, Thomas Klausner wrote: Modified Files: src: BUILDING Log Message: regen (using mandoc doc/BUILDING.mdoc > BUILDING -- let me know if a different way is preferred, I see that it now contains formatting). "make regen" in src/doc. If there's a mandoc equivalent for the ${TOOL_GROFF} command there, feel free to edit the Makefile. --apb (Alan Barrett)
Re: CVS commit: src/lib/libc/iconv
On Sat, 11 May 2013, Blue Rats wrote: .Fn iconv_close , and .Fn iconv -conform to +conforms to .St -p1003.1-2001 . "conform" was correct, and "conforms" is wrong. Please reinstate the correct wording. --apb (Alan Barrett)
Re: CVS commit: src/tools/host-mkdep
On Fri, 15 Mar 2013, Christos Zoulas wrote: I still don't understand how a missing include does not cause an error in the compilation phase, but it can be ignore in the dependency generation. tools/compat creates a dummy include file, if necessary. This is good enough for the compilation phase. I am not sure why it's not good enough for host-mkdep, but two ideas spring to mind: perhaps host-mkdep is run before the dummy include file is created, or perhaps host-mkdep is run without the -I flags that would allow it to find the dummy include file. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/make
On Tue, 05 Mar 2013, Christos Zoulas wrote: Modified Files: src/usr.bin/make: dir.c job.c job.h make.1 parse.c Log Message: Add a .STALE special target that gets invoked when dependency files contain stail entries. Please discuss this sort of thing first. There may be a way to solve the underlying problem without adding a new special target, such as using the newish "meta mode". Also, a better name than ".STALE" might have been suggested. --apb (Alan Barrett)
Re: CVS commit: src/distrib/sets/lists/base
On Sun, 03 Mar 2013, Alan Barrett wrote: Module Name:src Committed By: apb Date: Sun Mar 3 20:07:09 UTC 2013 Modified Files: src/distrib/sets/lists/base: mi Log Message: /tmp/cvs12835b I have updated the log message to say: Add zoneinfo files for new timezones from tzdata2013a: /usr/share/zoneinfo/Asia/Khandyga /usr/share/zoneinfo/Asia/Ust-Nera /usr/share/zoneinfo/Europe/Busingen
Re: CVS commit: src/usr.bin/vis
On Thu, 14 Feb 2013, Christos Zoulas wrote: Modified Files: src/usr.bin/vis: vis.c More fixes from J.R. Oldroyd: - In the call to strvisx() the count must be 1, not mbilen which can be 2 or 3 etc for a multibyte character. This value is a count of characters - not bytes - to process. It even says characters in the man page. In vis(3) I am interpreting this value to mean multibyte characters. In general, the caller of str[n]vis[x] knows how many bytes of data they have, but they do not know how many multibyte characters that might represent. If the man page talks about characters, that's because it was written at a time when vis did not attempt to deal with multibyte characters. I think that we should revert to the original semantics of lengths being measured in bytes, and adjust both the man pages and callers appropriately. --apb (Alan Barrett)
Re: CVS commit: src/sys/arch/sparc64
On Mon, 04 Feb 2013, Michael Lorenz wrote: Modified Files: src/sys/arch/sparc64/include: cpu.h src/sys/arch/sparc64/sparc64: machdep.c Log Message: add a sysctl.vis node that indicated which version of the VIS instruction set is supported. Currently this will be 1 for UltraSPARC I and II, 2 for UltraSPARC-III and up Does this need to use #define CPU_VIS, or could it use a new-style dynamic MIB entry, passing CTL_CREATE to sysctl_createv(9)? --apb (Alan Barrett)
Re: CVS commit: src/lib/librumpclient
On Thu, 17 Jan 2013, Antti Kantee wrote: Module Name:src Committed By: pooka Date: Thu Jan 17 20:47:44 UTC 2013 Modified Files: src/lib/librumpclient: rumpclient.c rumpclient.h Log Message: Solaris 10 fixes Please describe the actual changes. e.g. "use foo instead of bar". --apb (Alan Barrett)
Re: CVS commit: src/lib/libutil
On Mon, 31 Dec 2012, Joerg Sonnenberger wrote: Log Message: If malloc, calloc, or realloc returns NULL when a size of 0 was requested, which is allowed by pertinent standards, honor it instead of bombing. Do not do this for calloc(x, y) where x != 0 && y != 0 but x*y == 0; in that case bomb. The commit message is misleading. We expect calloc(x,y) to return NULL if x!=0 && y!=0 && x*y==0. (x!=0 && y!=0 && x*y==0) can be true only if calculating x*y results in what would loosely be called integer overflow; since the types are unsigned, it's a well-defined kind of wraparound, not the undefined kind of overflow. I'd expect an error like EINVAL rather than an error like ENOMEM for this case. --apb (Alan Barrett)
Re: CVS commit: src/usr.bin/units
On Sat, 29 Dec 2012, Robert Elz wrote: | because unit names cannot contain hyphens. Should that not be mentioned in the man page? Perhaps together with a suggestion that if the name the user requires should have hyphens, it simply be entered with the hyphens deleted ... perhaps even using tappit-hen as an example. Good idea. --apb (Alan Barrett)
Re: CVS commit: src/share/mk
On Mon, 10 Dec 2012, Antti Kantee wrote: +# If the toolchain does not know about a file, --print-file-name echoes +# the input file name (why??). In that case we want an empty string for +# dependencies to work correctly. Note: do _not_ use TOOL_SED here because +# this file is used before TOOL_SED is built. It's probably a bug if bsd.gcc.mk is used before TOOL_SED is built. However, you have to .include before you try to use any of its variables in a "!=" line. +_GCC_CRTBEGINS!= ${CC} --print-file-name=crtbeginS.o \ + | sed 's|^crtbeginS.o$$||' You can remove the dependency on sed by using make :C/// variable modifiers, like this (untested): _GCC_CRTBEGINS!=${CC} --print-file-name=crtbeginS.o _GCC_CRTENDS!= ${CC} --print-file-name=crtendS.o _GCC_CRTI!= ${CC} --print-file-name=crti.o _GCC_CRTN!= ${CC} --print-file-name=crtn.o .for v in _GCC_CRTBEGINS _GCC_CRTENDS _GCC_CRTI _GCC_CRTN # If the value does not contain '/' then replace it with the empty string ${v} := ${${v}:C,^[^/]*$,,} .endfor --apb (Alan Barrett)
Re: CVS commit: src/sys/sys
On Sat, 01 Dec 2012, Nick Hudson wrote: Log Message: Check for _NETBSD_SOURCE being defined wherever we check for _INCOMPLETE_XOPEN_C063 so that we expose the new POSIX extended API set recently added. The idea behind _INCOMPLETE_XOPEN_C063 was to make the incomplete implementation hidden by default, requiring explicit action to make it visible. If the implementation is now sufficiently complete that there is no longer a need for it to be hidden by default, then there is probably no need for the preprocessor symbol _INCOMPLETE_XOPEN_C063 to exist at all. On the other hand, if the implementation si still incomplete, then exposing it when _NETBSD_SOURCE is defined seems wrong. I don't see any third scenario in which it makes sense to test defined(_NETBSD_SOURCE) || defined(_INCOMPLETE_ --apb (Alan Barrett) From: Alan Barrett To: source-changes-d@NetBSD.org Cc: Bcc: Subject: Re: CVS commit: src/sys/sys Reply-To: In-Reply-To: <20121201082055.886f717...@cvs.netbsd.org> On Sat, 01 Dec 2012, Nick Hudson wrote: Log Message: Check for _NETBSD_SOURCE being defined wherever we check for _INCOMPLETE_XOPEN_C063 so that we expose the new POSIX extended API set recently added. This exposes the implementation by default. If the implementation is ready for that, then there is probably no need for the preprocessor symbol _INCOMPLETE_XOPEN_C063 to exist at all. --apb (Alan Barrett)