Re: CVS commit: src/sys/miscfs/kernfs
On Mon, Jul 19, 2021 at 01:33:53AM +, David A. Holland wrote: > Part 3; cvs randomly didn't commit all the files the first time, still > hunting down the files it skipped. ok, I'm pretty sure that's the lot. -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src
On Fri, 16 Jul 2021 12:44:18 +0200 Frank Kardel wrote: > I am not sure why NO_FOLLOW was there and if we want to keep it how > > we would handle named wedges. We could create a /dev/wedgedev from > > devpubd with hard links to the devices - that would work with ZFS but > > is really ugly. > > Comments? When using ZFS on Linux it is quite normal and expected to have pool components that are symlinks. /dev/disk/by-uuid/u -> /dev/sdb et cetera. I also have a NetBSD system where I have manually created such symlinks for the components, so I would much prefer it if this continues to work as before.
Re: CVS commit: src
Hi ! This triggered an unexpected side effect. I have my ZFS vdev on gpt partitions as wedges. I configured the vdevs via the /dev/wedges/* symlinks via the wedge names. data 1.28T 424G 888G - 4% 32% 1.31x ONLINE - raidz2 1.28T 424G 888G - 4%32% wedges/zfs440g00 - - - - - - wedges/zfs440g10 - - - - - - wedges/zfs440g20 - - - - - - wedges/zfs440g30 - - - - - - wedges/zfs440g40 - - - - - - wedges/zfs440g50 - - - - - - cache - - - - - - wedges/zfsnv1c01 128G 4.64M 128G - 0% 0% This used to work in 9.99.85 (devpubd needs to run before zfs though). In 9.99.86 the NO_FOLLOW is now being honored the above strategy fails and leads to UNAVAIL pools. Importing the pools via /dev/dk* works but is not useful as the numbering of the dk slices can easily change. Uncommenting "filemode |= O_NOFOLLOW;" makes things work again. I am not sure why NO_FOLLOW was there and if we want to keep it how we would handle named wedges. We could create a /dev/wedgedev from devpubd with hard links to the devices - that would work with ZFS but is really ugly. Comments? Frank On 06/30/21 00:40, David A. Holland wrote: Module Name:src Committed By: dholland Date: Tue Jun 29 22:40:54 UTC 2021 Modified Files: src/external/cddl/osnet/sys/sys: vnode.h src/share/man/man9: errno.9 vnsubr.9 src/sys/dev: firmload.c fss.c kloader.c vnd.c src/sys/kern: kern_acct.c kern_core.c kern_descrip.c kern_ktrace_vfs.c kern_module_vfs.c subr_exec_fd.c subr_kobj_vfs.c tty_ptm.c vfs_syscalls.c vfs_vnops.c src/sys/miscfs/fdesc: fdesc_vnops.c src/sys/modules/lua: lua.c src/sys/sys: filedesc.h vnode.h src/sys/ufs/lfs: ulfs_extattr.c ulfs_quota1.c src/sys/ufs/ufs: ufs_extattr.c ufs_quota1.c Log Message: Add containment for the cloning devices hack in vn_open. Cloning devices (and also things like /dev/stderr) work by allocating a struct file, stuffing it in the file table (which is a layer violation), stuffing the file descriptor number for it in a magic field of struct lwp (which is gross), and then "failing" with one of two magic errnos, EDUPFD or EMOVEFD. Before this commit, all callers of vn_open in the kernel (there are quite a few) were expected to check for these errors and handle the situation. Needless to say, none of them except for open() itself did, resulting in internal negative errnos being returned to userspace. This hack is fairly deeply rooted and cannot be eliminated all at once. This commit adds logic to handle the magic errnos inside vn_open; now on success vn_open returns either a vnode or an integer file descriptor, along with a flag that says whether the underlying code requested EDUPFD or EMOVEFD. Callers not prepared to cope with file descriptors can pass NULL for the extra return values, in which case if a file descriptor would be produced vn_open fails with EOPNOTSUPP. Since I'm rearranging vn_open's signature anyway, stop exposing struct nameidata. Instead, take three arguments: an optional vnode to use as the starting point (like openat()), the path, and additional namei flags to use, restricted to NOCHROOT and TRYEMULROOT. (Other namei behavior, e.g. NOFOLLOW, can be requested via the open flags.) This change requires a kernel bump. Ride the one an hour ago. (That was supposed to be coordinated; did not intend to let an hour slip by. My fault.) To generate a diff of this commit: cvs rdiff -u -r1.18 -r1.19 src/external/cddl/osnet/sys/sys/vnode.h cvs rdiff -u -r1.5 -r1.6 src/share/man/man9/errno.9 cvs rdiff -u -r1.47 -r1.48 src/share/man/man9/vnsubr.9 cvs rdiff -u -r1.22 -r1.23 src/sys/dev/firmload.c cvs rdiff -u -r1.110 -r1.111 src/sys/dev/fss.c cvs rdiff -u -r1.28 -r1.29 src/sys/dev/kloader.c cvs rdiff -u -r1.281 -r1.282 src/sys/dev/vnd.c cvs rdiff -u -r1.97 -r1.98 src/sys/kern/kern_acct.c cvs rdiff -u -r1.34 -r1.35 src/sys/kern/kern_core.c cvs rdiff -u -r1.250 -r1.251 src/sys/kern/kern_descrip.c cvs rdiff -u -r1.2 -r1.3 src/sys/kern/kern_ktrace_vfs.c cvs rdiff -u -r1.17 -r1.18 src/sys/kern/kern_module_vfs.c cvs rdiff -u -r1.11 -r1.12 src/sys/kern/subr_exec_fd.c \ src/sys/kern/subr_kobj_vfs.c cvs rdiff -u -r1.42 -r1.43 src/sys/kern/tty_ptm.c cvs rdiff -u -r1.549 -r1.550 src/sys/kern/vfs_syscalls.c cvs rdiff -u -r1.215 -r1.216 src/sys/kern/vfs_vnops.c cvs rdiff -u -r1.137 -r1.138 src/sys/miscfs/fdesc/fdesc_vnops.c cvs rdiff -u -r1.24 -r1.25 src/sys/modules/lua/lua.c cvs rdiff -u -r1.68 -r1.69 src/sys/sys/filedesc.h cvs rdiff -u -r1.296 -r1.297 src/sys/sys/vnode.h cvs rdiff -u -r1.16 -r1.17 src/sys/ufs/lfs/ulfs_extattr.c cvs rdiff -u
Re: CVS commit: src/sys/dev/usb
On 15/07/2021 04:25, Tohru Nishimura wrote: Module Name:src Committed By: nisimura Date: Thu Jul 15 03:25:50 UTC 2021 Modified Files: src/sys/dev/usb: if_mue.c uchcom.c Log Message: explanation typo To generate a diff of this commit: cvs rdiff -u -r1.60 -r1.61 src/sys/dev/usb/if_mue.c cvs rdiff -u -r1.38 -r1.39 src/sys/dev/usb/uchcom.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/dev/usb/if_mue.c diff -u src/sys/dev/usb/if_mue.c:1.60 src/sys/dev/usb/if_mue.c:1.61 --- src/sys/dev/usb/if_mue.c:1.60 Sat Jun 27 13:33:26 2020 +++ src/sys/dev/usb/if_mue.cThu Jul 15 03:25:50 2021 [snip] did you mean to commit the if_mue.c change? Certainly the commit message doesn't match. Thanks, Nick
Re: CVS commit: src/external/gpl3/gcc/dist/gcc/ginclude
> On Jul 15, 2021, at 12:06 AM, matthew green wrote: > > "Christos Zoulas" writes: >> Module Name: src >> Committed By:christos >> Date:Wed Jul 14 13:24:59 UTC 2021 >> >> Modified Files: >> src/external/gpl3/gcc/dist/gcc/ginclude: stddef.h >> >> Log Message: >> clang does not support __float128 in our configuration and i386 > > shouldn't you replace this with some other 128-bit aligned > value then, instead of eliding it entirely? Good idea! Let me do that. christos signature.asc Description: Message signed with OpenPGP
re: CVS commit: src/external/gpl3/gcc/dist/gcc/ginclude
"Christos Zoulas" writes: > Module Name: src > Committed By: christos > Date: Wed Jul 14 13:24:59 UTC 2021 > > Modified Files: > src/external/gpl3/gcc/dist/gcc/ginclude: stddef.h > > Log Message: > clang does not support __float128 in our configuration and i386 shouldn't you replace this with some other 128-bit aligned value then, instead of eliding it entirely? .mrg.
Re: CVS commit: src/common/lib/libc/arch/aarch64/atomic
On 04/07/2021 23:11, Joerg Sonnenberger wrote:
On Sun, Jul 04, 2021 at 06:55:47AM +, Nick Hudson wrote:
Module Name:src
Committed By: skrll
Date: Sun Jul 4 06:55:47 UTC 2021
Modified Files:
src/common/lib/libc/arch/aarch64/atomic: atomic_nand_16.S
atomic_nand_32.S atomic_nand_64.S atomic_nand_8.S
Log Message:
Fix the logic operation for atomic_nand_{8,16,32,64}
From the gcc docs the operations are as follows
{ tmp = *ptr; *ptr = ~(tmp & value); return tmp; } // nand
{ tmp = ~(*ptr & value); *ptr = tmp; return *ptr; } // nand
yes, this is really rather strange.
This depends on which GCC version you are looking at. They changed it
sometime in the early GCC 4 days.
Right... gcc 4.4 which is well before NetBSD had an arm64/aarch64 port.
Nick
Re: CVS commit: src/common/lib/libc/arch/aarch64/atomic
On Sun, Jul 04, 2021 at 06:55:47AM +, Nick Hudson wrote:
> Module Name: src
> Committed By: skrll
> Date: Sun Jul 4 06:55:47 UTC 2021
>
> Modified Files:
> src/common/lib/libc/arch/aarch64/atomic: atomic_nand_16.S
> atomic_nand_32.S atomic_nand_64.S atomic_nand_8.S
>
> Log Message:
> Fix the logic operation for atomic_nand_{8,16,32,64}
>
> From the gcc docs the operations are as follows
>
> { tmp = *ptr; *ptr = ~(tmp & value); return tmp; } // nand
> { tmp = ~(*ptr & value); *ptr = tmp; return *ptr; } // nand
>
> yes, this is really rather strange.
This depends on which GCC version you are looking at. They changed it
sometime in the early GCC 4 days.
Joerg
Re: CVS commit: src/sys/kern
On Thu, Jul 01, 2021 at 12:25:51AM -0400, Christos Zoulas wrote: > Modified Files: > src/sys/kern: vfs_vnops.c > > Log Message: > don't clear the error before we use it to determine if we are moving or > duping. oh ffs... *goes to soak head* -- David A. Holland dholl...@netbsd.org
Re: CVS commit: src
On Wed, Jun 30, 2021 at 06:42:21PM +0900, Rin Okuyama wrote:
> /home/source/ab/HEAD/src/sys/kern/vfs_vnops.c:357:11: error: 'vp' may be
> used uninitialized in this function [-Werror=maybe-uninitialized]
> 357 | *ret_vp = vp;
> | ^~~~
> cc1: all warnings being treated as errors
If this still occurs (may depend on gcc version as usual), the fix is:
Index: sys/kern/vfs_vnops.c
===
RCS file: /cvsroot/src/sys/kern/vfs_vnops.c,v
retrieving revision 1.218
diff -u -p -r1.218 vfs_vnops.c
--- sys/kern/vfs_vnops.c30 Jun 2021 17:51:49 - 1.218
+++ sys/kern/vfs_vnops.c30 Jun 2021 17:53:56 -
@@ -214,8 +214,10 @@ vn_open(struct vnode *at_dvp, struct pat
l->l_dupfd = 0;
error = namei();
- if (error)
+ if (error) {
+ vp = NULL;
goto out;
+ }
vp = nd.ni_vp;
This is the only remaining path I see that the compiler might
reasonably get confused about.
--
David A. Holland
dholl...@netbsd.org
Re: CVS commit: src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm
On 30/06/2021 15:13, Joerg Sonnenberger wrote: On Tue, Jun 29, 2021 at 11:26:00PM +, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Tue Jun 29 23:26:00 UTC 2021 Modified Files: src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm: aeabi_cfcmp.S divmodsi4.S divsi3.S modsi3.S Log Message: Align sp to 8-byte boundary as required by EABI. Why not just push/pop another register? That's how it is normally done for generated code. https://mail-index.netbsd.org/port-arm/2021/06/30/msg007341.html Nick
Re: CVS commit: src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm
On Tue, Jun 29, 2021 at 11:26:00PM +, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Tue Jun 29 23:26:00 UTC 2021 > > Modified Files: > src/sys/external/bsd/compiler_rt/dist/lib/builtins/arm: aeabi_cfcmp.S > divmodsi4.S divsi3.S modsi3.S > > Log Message: > Align sp to 8-byte boundary as required by EABI. Why not just push/pop another register? That's how it is normally done for generated code. Joerg
Re: CVS commit: src
On Tue, Jun 29, 2021 at 10:40:54PM +, David A. Holland wrote: > Module Name: src > Committed By: dholland > Date: Tue Jun 29 22:40:54 UTC 2021 > > Modified Files: > src/external/cddl/osnet/sys/sys: vnode.h > src/share/man/man9: errno.9 vnsubr.9 > src/sys/dev: firmload.c fss.c kloader.c vnd.c > src/sys/kern: kern_acct.c kern_core.c kern_descrip.c kern_ktrace_vfs.c > kern_module_vfs.c subr_exec_fd.c subr_kobj_vfs.c tty_ptm.c > vfs_syscalls.c vfs_vnops.c > src/sys/miscfs/fdesc: fdesc_vnops.c > src/sys/modules/lua: lua.c > src/sys/sys: filedesc.h vnode.h > src/sys/ufs/lfs: ulfs_extattr.c ulfs_quota1.c > src/sys/ufs/ufs: ufs_extattr.c ufs_quota1.c > > Log Message: > Add containment for the cloning devices hack in vn_open. > > Cloning devices (and also things like /dev/stderr) work by allocating > a struct file, stuffing it in the file table (which is a layer > violation), stuffing the file descriptor number for it in a magic > field of struct lwp (which is gross), and then "failing" with one of > two magic errnos, EDUPFD or EMOVEFD. bpf seems to be completely broken after this change. At least for me on amd64 with a monolithic kernel (neither dhcpcd nor wpa_supplicant work). Martin
Re: CVS commit: src
Hi, GCC warns maybe-uninitialized for this commit (for some architectures and optimization levels: https://releng.netbsd.org/builds/HEAD/202106292250Z/): /home/source/ab/HEAD/src/sys/kern/vfs_vnops.c: In function 'vn_open': /home/source/ab/HEAD/src/sys/kern/vfs_vnops.c:357:11: error: 'vp' may be used uninitialized in this function [-Werror=maybe-uninitialized] 357 | *ret_vp = vp; | ^~~~ cc1: all warnings being treated as errors Can you please double check and work around if no problem? Thanks, rin On 2021/06/30 7:40, David A. Holland wrote: Module Name:src Committed By: dholland Date: Tue Jun 29 22:40:54 UTC 2021 Modified Files: src/external/cddl/osnet/sys/sys: vnode.h src/share/man/man9: errno.9 vnsubr.9 src/sys/dev: firmload.c fss.c kloader.c vnd.c src/sys/kern: kern_acct.c kern_core.c kern_descrip.c kern_ktrace_vfs.c kern_module_vfs.c subr_exec_fd.c subr_kobj_vfs.c tty_ptm.c vfs_syscalls.c vfs_vnops.c src/sys/miscfs/fdesc: fdesc_vnops.c src/sys/modules/lua: lua.c src/sys/sys: filedesc.h vnode.h src/sys/ufs/lfs: ulfs_extattr.c ulfs_quota1.c src/sys/ufs/ufs: ufs_extattr.c ufs_quota1.c Log Message: Add containment for the cloning devices hack in vn_open. Cloning devices (and also things like /dev/stderr) work by allocating a struct file, stuffing it in the file table (which is a layer violation), stuffing the file descriptor number for it in a magic field of struct lwp (which is gross), and then "failing" with one of two magic errnos, EDUPFD or EMOVEFD. Before this commit, all callers of vn_open in the kernel (there are quite a few) were expected to check for these errors and handle the situation. Needless to say, none of them except for open() itself did, resulting in internal negative errnos being returned to userspace. This hack is fairly deeply rooted and cannot be eliminated all at once. This commit adds logic to handle the magic errnos inside vn_open; now on success vn_open returns either a vnode or an integer file descriptor, along with a flag that says whether the underlying code requested EDUPFD or EMOVEFD. Callers not prepared to cope with file descriptors can pass NULL for the extra return values, in which case if a file descriptor would be produced vn_open fails with EOPNOTSUPP. Since I'm rearranging vn_open's signature anyway, stop exposing struct nameidata. Instead, take three arguments: an optional vnode to use as the starting point (like openat()), the path, and additional namei flags to use, restricted to NOCHROOT and TRYEMULROOT. (Other namei behavior, e.g. NOFOLLOW, can be requested via the open flags.) This change requires a kernel bump. Ride the one an hour ago. (That was supposed to be coordinated; did not intend to let an hour slip by. My fault.) To generate a diff of this commit: cvs rdiff -u -r1.18 -r1.19 src/external/cddl/osnet/sys/sys/vnode.h cvs rdiff -u -r1.5 -r1.6 src/share/man/man9/errno.9 cvs rdiff -u -r1.47 -r1.48 src/share/man/man9/vnsubr.9 cvs rdiff -u -r1.22 -r1.23 src/sys/dev/firmload.c cvs rdiff -u -r1.110 -r1.111 src/sys/dev/fss.c cvs rdiff -u -r1.28 -r1.29 src/sys/dev/kloader.c cvs rdiff -u -r1.281 -r1.282 src/sys/dev/vnd.c cvs rdiff -u -r1.97 -r1.98 src/sys/kern/kern_acct.c cvs rdiff -u -r1.34 -r1.35 src/sys/kern/kern_core.c cvs rdiff -u -r1.250 -r1.251 src/sys/kern/kern_descrip.c cvs rdiff -u -r1.2 -r1.3 src/sys/kern/kern_ktrace_vfs.c cvs rdiff -u -r1.17 -r1.18 src/sys/kern/kern_module_vfs.c cvs rdiff -u -r1.11 -r1.12 src/sys/kern/subr_exec_fd.c \ src/sys/kern/subr_kobj_vfs.c cvs rdiff -u -r1.42 -r1.43 src/sys/kern/tty_ptm.c cvs rdiff -u -r1.549 -r1.550 src/sys/kern/vfs_syscalls.c cvs rdiff -u -r1.215 -r1.216 src/sys/kern/vfs_vnops.c cvs rdiff -u -r1.137 -r1.138 src/sys/miscfs/fdesc/fdesc_vnops.c cvs rdiff -u -r1.24 -r1.25 src/sys/modules/lua/lua.c cvs rdiff -u -r1.68 -r1.69 src/sys/sys/filedesc.h cvs rdiff -u -r1.296 -r1.297 src/sys/sys/vnode.h cvs rdiff -u -r1.16 -r1.17 src/sys/ufs/lfs/ulfs_extattr.c cvs rdiff -u -r1.11 -r1.12 src/sys/ufs/lfs/ulfs_quota1.c cvs rdiff -u -r1.52 -r1.53 src/sys/ufs/ufs/ufs_extattr.c cvs rdiff -u -r1.23 -r1.24 src/sys/ufs/ufs/ufs_quota1.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src
On Tue, 29 Jun 2021, Paul Goyette wrote: Please note that both riastradh@ and dh@ have plans to ride this version bump. You might want to delay any ``cvs up'' commands for a short while. :) Looks like both commits are done, so ``cvs up'' should get you a workable source tree. ++--+--+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses:| | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com| | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | || | pgoyett...@gmail.com | ++--+--+
Re: CVS commit: src/sys
On Tue, 29 Jun 2021, David A. Holland wrote: Module Name:src Committed By: dholland Date: Tue Jun 29 22:39:21 UTC 2021 Modified Files: src/sys/fs/puffs: puffs_vnops.c src/sys/fs/union: union_vnops.c src/sys/fs/unionfs: unionfs_subr.c src/sys/kern: vfs_getcwd.c vfs_lookup.c src/sys/sys: namei.src Log Message: Now remove cn_consume from struct componentname. This change requires a kernel bump. Note that we're riding the 9.99.86 bump from earlier today. Note though that I'm not going to version the VOP_LOOKUP args structure (or any other args structure) as code that doesn't touch cn_consume doesn't need attention and code that does will fail on it without further intervention. To generate a diff of this commit: cvs rdiff -u -r1.218 -r1.219 src/sys/fs/puffs/puffs_vnops.c cvs rdiff -u -r1.76 -r1.77 src/sys/fs/union/union_vnops.c cvs rdiff -u -r1.14 -r1.15 src/sys/fs/unionfs/unionfs_subr.c cvs rdiff -u -r1.60 -r1.61 src/sys/kern/vfs_getcwd.c cvs rdiff -u -r1.228 -r1.229 src/sys/kern/vfs_lookup.c cvs rdiff -u -r1.59 -r1.60 src/sys/sys/namei.src Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. !DSPAM:60dba34c228001125714949! ++--+--+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses:| | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com| | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | || | pgoyett...@gmail.com | ++--+--+
Re: CVS commit: src/lib/libc/arch/arm
Oops, this is broken. I will fix shortly... rin On 2021/06/30 8:29, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Tue Jun 29 23:29:12 UTC 2021 Modified Files: src/lib/libc/arch/arm/gen: swapcontext.S src/lib/libc/arch/arm/sys: __clone.S Log Message: Align sp to 8-byte boundary as required by EABI. IIUC, this change only affects libc compiled for ``Thumb-mode userland'', which we've not officially supported yet. To generate a diff of this commit: cvs rdiff -u -r1.15 -r1.16 src/lib/libc/arch/arm/gen/swapcontext.S cvs rdiff -u -r1.9 -r1.10 src/lib/libc/arch/arm/sys/__clone.S Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src
Please note that both riastradh@ and dh@ have plans to ride this
version bump. You might want to delay any ``cvs up'' commands for
a short while.
:)
On Tue, 29 Jun 2021, Paul Goyette wrote:
Module Name:src
Committed By: pgoyette
Date: Tue Jun 29 21:03:37 UTC 2021
Modified Files:
src/lib/libpci: Makefile
src/sys/arch/arm/amlogic: gxlphy.c
src/sys/dev: dev_verbose.c dev_verbose.h devlist2h.awk
src/sys/dev/hdaudio: hdaudio.c
src/sys/dev/mii: Makefile.miidevs gentbi.c mii_physubr.c mii_verbose.c
mii_verbose.h miivar.h ukphy.c
src/sys/dev/pci: pci_subr.c
src/sys/dev/pci/ixgbe: ixgbe.c
src/sys/rump/librump/rumpdev: Makefile.rumpdev
src/sys/sys: param.h
Removed Files:
src/sys/dev/mii: devlist2h.awk
Log Message:
Rework the xxxVERBOSE option to share the common module-hook-based
verbose mechanism with MIIVERBOSE. This reduces some duplicated code
and allows us to once again permit auto-unload of MIIVERBOSE.
Change details:
* Update dev/devlist2h.awk to accomodate miidevs, including generation
of MII_STR_oui_model definitions and use of oui and model rather than
vendor and product. This also changes the compressed data in the
xxxdevs_data.h files to uint32_t (since mii oui's are up to 6 hex
digits long)
* Update a couple of phy drivers to use new calls to get verbose data
* Regen all of the xxxdevs{,_data}.h files (separate commit, coming
very soon)
* Update mii/mii_verbose.[ch] and mii/mii_physubr.c to use the various
DEV_VERBOSE_xxx macros
* Update the pci, usb, and hdaudio code as needed, to #include the
xxxdevs.h files (in order to get the proper printf format strings)
* Since dev/dev_verbose.c now uses non-literal printf format strings,
(to deal with the vendor/product vs oui/model issue), we need to
make sure it gets compiled with -Wno-error=format-nonliteral, even
in userland's libpci and librumpdev!
* Bump kernel version for the change in module interfaces
Welcome to 9.99.86!
XXX It might be useful in the future to extend the MII_STR_oui_model
XXX definitions to PCI as well (and perhaps USB and HDAUDIO). This
XXX would allow for a single centralized location for the products'
XXX descriptions, rather than being dispersed among individual
XXX drivers' xxx_match tables.
To generate a diff of this commit:
cvs rdiff -u -r1.5 -r1.6 src/lib/libpci/Makefile
cvs rdiff -u -r1.4 -r1.5 src/sys/arch/arm/amlogic/gxlphy.c
cvs rdiff -u -r1.3 -r1.4 src/sys/dev/dev_verbose.c src/sys/dev/devlist2h.awk
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/dev_verbose.h
cvs rdiff -u -r1.14 -r1.15 src/sys/dev/hdaudio/hdaudio.c
cvs rdiff -u -r1.3 -r1.4 src/sys/dev/mii/Makefile.miidevs
cvs rdiff -u -r1.11 -r0 src/sys/dev/mii/devlist2h.awk
cvs rdiff -u -r1.31 -r1.32 src/sys/dev/mii/gentbi.c
cvs rdiff -u -r1.94 -r1.95 src/sys/dev/mii/mii_physubr.c
cvs rdiff -u -r1.8 -r1.9 src/sys/dev/mii/mii_verbose.c
cvs rdiff -u -r1.2 -r1.3 src/sys/dev/mii/mii_verbose.h
cvs rdiff -u -r1.73 -r1.74 src/sys/dev/mii/miivar.h
cvs rdiff -u -r1.54 -r1.55 src/sys/dev/mii/ukphy.c
cvs rdiff -u -r1.225 -r1.226 src/sys/dev/pci/pci_subr.c
cvs rdiff -u -r1.284 -r1.285 src/sys/dev/pci/ixgbe/ixgbe.c
cvs rdiff -u -r1.13 -r1.14 src/sys/rump/librump/rumpdev/Makefile.rumpdev
cvs rdiff -u -r1.696 -r1.697 src/sys/sys/param.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
!DSPAM:60db8aaf294301898520769!
++--+--+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses:|
| (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com|
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org |
|| | pgoyett...@gmail.com |
++--+--+
Re: CVS commit: src/lib/libcurses
On Wed, Jun 23, 2021 at 07:29:30AM +, Martin Husemann wrote: > This (or the series) broke the non-wide char builds. > Easy/quick reproducer: build.sh -m sun2 release /home/builds/ab/HEAD/sun3/202106222020Z-tools/lib/gcc/m68k--netbsdelf/10.3.0/../../../../m68k--netbsdelf/bin/ld: libhack.o: in function `__slk_draw': /usr/src/distrib/utils/libhack/../../../lib/libcurses/slk.c:860: undefined reference to `setcchar' /home/builds/ab/HEAD/sun3/202106222020Z-tools/lib/gcc/m68k--netbsdelf/10.3.0/../../../../m68k--netbsdelf/bin/ld: /usr/src/distrib/utils/libhack/../../../lib/libcurses/slk.c:870: undefined reference to `mvwins_wch' /home/builds/ab/HEAD/sun3/202106222020Z-tools/lib/gcc/m68k--netbsdelf/10.3.0/../../../../m68k--netbsdelf/bin/ld: /usr/src/distrib/utils/libhack/../../../lib/libcurses/slk.c:873: undefined reference to `mvwadd_wch' collect2: error: ld returned 1 exit status See for example: http://releng.netbsd.org/builds/HEAD/202106222020Z/sun3.build.failed Martin
Re: CVS commit: src/lib/libcurses
On Tue, Jun 22, 2021 at 07:49:09AM +, Brett Lymn wrote: > Module Name: src > Committed By: blymn > Date: Tue Jun 22 07:49:09 UTC 2021 > > Modified Files: > src/lib/libcurses: addbytes.c > > Log Message: > Rework the fix for lib/56224. > Move the scroll check to _cursesi_addwchar > Perform the scroll check before updating the cursor location when > processing \n. > > > To generate a diff of this commit: > cvs rdiff -u -r1.56 -r1.57 src/lib/libcurses/addbytes.c This (or the series) broke the non-wide char builds. Easy/quick reproducer: build.sh -m sun2 release Can you please fix? Thanks! Martin
Re: CVS commit: src/sys/dev/pad
On 2021/06/16 19:23, Rin Okuyama wrote: On 2021/06/16 18:15, Taylor R Campbell wrote: Date: Wed, 16 Jun 2021 17:38:26 +0900 From: Rin Okuyama KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi. Thanks for hints! Seems like kernel module bug for powerpc. pad(4) is not in GENERIC. If it is in kernel, panic does not take place. On the other hand, if it is supplied as module, even if its pad.c (and subr_autoconf.c in main kernel) is up to date, panic occurs. Another problem is kernel modules for powerpc is: http://mail-index.netbsd.org/port-powerpc/2020/07/07/msg003590.html I still haven't figured out why... This turned out to be an MI bug between UP kernel v.s. modules: http://mail-index.netbsd.org/source-changes/2021/06/16/msg130239.html Should be fixed now :). Thanks, rin
Re: CVS commit: src/sys/dev/pad
On 2021/06/16 18:15, Taylor R Campbell wrote: Date: Wed, 16 Jun 2021 17:38:26 +0900 From: Rin Okuyama KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi. Thanks for hints! Seems like kernel module bug for powerpc. pad(4) is not in GENERIC. If it is in kernel, panic does not take place. On the other hand, if it is supplied as module, even if its pad.c (and subr_autoconf.c in main kernel) is up to date, panic occurs. Another problem is kernel modules for powerpc is: http://mail-index.netbsd.org/port-powerpc/2020/07/07/msg003590.html I still haven't figured out why... Thanks, rin
Re: CVS commit: src/sys/dev/pad
> Date: Wed, 16 Jun 2021 17:38:26 +0900 > From: Rin Okuyama > > KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): > [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file > "/usr/src/sys/dev/pad/pad.c", line 214 Can you share `ident netbsd | grep -e pad.c -e subr_autoconf.c' output? config_attach_pseudo takes the kernel lock, so this assertion should definitely not fire. > I don't understand why this happens on macppc, while does not on > majority of other machines. Can this behavior depend on underlying > audio(4) driver? If so, what should I do to fix? I don't think so -- this happens before we even reach audio_attach_mi.
Re: CVS commit: src/sys/dev/pad
Hi, KASSERT added to pad_attach() by this commit fires on macppc with mixerctl(1): # cd /usr/tests/usr.bin/mixerctl && atf-run ... tc-start: ..., nflag [...] panic: kernel diagnostic assertion "KERNEL_LOCKED_P()" failed: file "/usr/src/sys/dev/pad/pad.c", line 214 [...] cpu0: Begin traceback... [...] ... vpanic ... [...] ... kern_assert ... [...] ... pad_attach ... [...] ... config_attach_pseudo ... [...] ... pad_open ... [...] ... spec_open ... [...] ... VOP_OPEN ... [...] ... vn_open ... [...] ... do_open ... [...] ... do_sys_openat ... [...] ... sys_open ... [...] ... syscall ... [...] user SC trap #5 by ... (copy from framebuffer console by hands) I don't understand why this happens on macppc, while does not on majority of other machines. Can this behavior depend on underlying audio(4) driver? If so, what should I do to fix? Thanks, rin On 2021/06/14 19:14, Taylor R Campbell wrote: Module Name:src Committed By: riastradh Date: Mon Jun 14 10:14:58 UTC 2021 Modified Files: src/sys/dev/pad: pad.c Log Message: pad(4): Make this exclusively a cloning device. padN numbering never corresponded with audioM numbering except by accident, so the non-cloning device never worked reliably for scripting. This simplifies the logic substantially. While here, fix drvctl detach race. To generate a diff of this commit: cvs rdiff -u -r1.70 -r1.71 src/sys/dev/pad/pad.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: vfork() and posix_spawn() [was: Re: CVS commit: src/lib/libc/sys]
Date:Mon, 14 Jun 2021 03:56:48 +0200 From:Joerg Sonnenberger Message-ID: | This is even more true for multi-threaded applications | (where POSIX explicitly suggests that requirement). Sure, anything with fork() and threads has issues, that's messy. Even I know that, and I know very little about threads. | On the specific topic, I'm somewhat puzzled by the claim that fork is | async-signal-safe That's not what I said, I said it isn't (though the way I phrased that might not have been all that clear - I know I often type too much, and sometimes overcompensate by typing too little). But (from my message): If the "that said" relates to fork() or vfork() not being async signal safe, so a double fork() (when the first is vfork anyway) would not be condoned, was meant to acknowledge that fork() (and vfork()) aren't async signal safe, and so if a requirement that async signal safe functions are all that is permitted after vfork() actually existed, then a if (vfork() == 0) fork(); sequence would not be permitted (not generate defined actions). But for the purposes of whether double fork() option to posix_spawn is useful or not, this restriction doesn't matter, as the above can be (eventually) written if (vfork() == 0) _Fork(); instead, as _Fork() is (to be) async signal safe. Same effect as the (perhaps) undefined version above for this purpose. | since most implementations will require synchronisation for pthread_atfork Yes, which is what _Fork() does not do (_Fork() is to fork() as _exit() is to exit() - in a sense). | It most likely should. The main reason is that much of the system can | and often do depend on things like mutexes to ensure correctness and all | that is essentially UB after vfork(). Actually, it isn't UB, or shouldn't be. The behaviour of vfork() is actually very precisely specified (which doesn't take much, as it is quite simple). There's very little room for UB. What there is is plenty of room for simple errors to screw things (and appear as if it is UB, when it is actually quite well defined broken code). There's no problem using mutexes after a vfork() with two caveats - first anything locked must be unlocked again before the eventual _exit() or exec() (the mutex must be returned to its state at the time of the vfork()) and the child must not need to lock anything which might be already locked in the parent (as that's a guaranteed deadlock). How one makes the latter condition work is for someone else to solve... That said, most of what would require a mutex isn't something that should be being done after a vfork() anyway - most of those operations are likely to be things which change state of the process, and that's what (at least without careful preparation) cannot happen in a child of a vfork() (so no use of stdio, no use of malloc, ...) | That's even ignoring the stronger issue of mutating state. Yes, though if it is intentional, that can actually be OK (sometimes). Our /bin/sh relies upon the ability to modify its parent's state after a vfork() to communicate status back to the parent (more than is possible reliably with exit status - as the parent cannot tell from that whether the status it receives is from its own image running the child after the vfork() or from some other process after an exec() succeeded). A modified volatile variable (in the parent), however, can only have been modified by the vfork() child (we assume here that the parent isn't modifying the variable itself elsewhere, naturally). And yes, that means that on any system that "implements" vfork() using cc -Dvfork=fork (or its equivalent) cannot use vfork() with our sh - we need a "real" vfork, or must simply disable its use and only use fork (which is what the -D implementation does anyway of course, just badly). | vfork use really should die... That sentiment has been around for a long time - almost since vfork() was first created (40+ years ago). But it is still here. Implementing fork() using CoW was supposed to solve all the issues with fork(). It didn't, vfork() is still lots faster. posix_spawn() might allow some uses of vfork()+exec() to be retired, that would be good, but it isn't going to get all of them. | No, it relates to one common pattern for used by or for daemon. Yes, I understood that, but why do we care? Daemons start how often? What percentage of the forks() (vfork is, in practice, never used for this) on your systems are generated by daemons starting? What kind of saving would you expect to see from allowing posix_spawn() to replace that fork();fork() sequence ? Is it really worth even the tiniest extra complexity in that already fairly complex interface to handle this almost irrelevant issue? Do you believe in non-memory managed real time systems there are any daemon processes like that at all? | There are non-trivial uses of fork, yes. There are much less
Re: vfork() and posix_spawn() [was: Re: CVS commit: src/lib/libc/sys]
On Mon, Jun 14, 2021 at 01:33:51AM +0700, Robert Elz wrote: > Date:Sat, 12 Jun 2021 23:13:54 +0200 > From:Joerg Sonnenberger > Message-ID: > > Sorry, missed this message when I was cherry picking messages to read in > a timely fashion. > > | On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote: > | > after a vfork() the child can do anything > > | This is not true. After vfork(), it is only supposed to call async > | signal safe functions. > > What you said, and what I said are not contradictory. Note I said "can > do" and you said "is only supposed to". > > What is supposed to work and what actually does work (or can be made to > work) are two different things. Sure, but the set of what works reliably has actually been shrinking over time and more bugs in various programs using vfork specifically have been discovered. This is even more true for multi-threaded applications (where POSIX explicitly suggests that requirement). On the specific topic, I'm somewhat puzzled by the claim that fork is async-signal-safe since most implementations will require synchronisation for pthread_atfork and that seems to place quite a burden on the implementation... > I would note though our man page doesn't make that requirement, or not > that I can see anyway, and vfork() is not in POSIX of course - and while > I don't have a copy to check, I'd suspect not in the C standards either), > so while that sounds like a reasonable requirement to impose for safer > operation, I'm not sure that anyone or anything actually does that. It most likely should. The main reason is that much of the system can and often do depend on things like mutexes to ensure correctness and all that is essentially UB after vfork(). That's even ignoring the stronger issue of mutating state. vfork use really should die... > > | That said, a flag for the double fork semantic might be useful. > > If the "that said" relates to fork() or vfork() not being async signal safe, > so a double fork() (when the first is vfork anyway) would not be condoned, > that doesn't matter, there to be an async-signal-safe _Fork() call added to > the next version of POSIX, so even with the "only async signal safe" > requirement for vfork() a: No, it relates to one common pattern for used by or for daemon. > Please don't see posix_spawn() as being (or ever becoming) a panacea that > can replace all fork() (or even just vfork()) calls - even just in the > cases where an exec() follows soon after. It works fine for most simple > cases, but there are lots of not-so-simple situations that it cannot handle, > and burdening the interface with the ability to deal with all of those > would reduce the "efficiently implementable" goal for little real benefit. There are non-trivial uses of fork, yes. There are much less non-trivial uses of vfork as the latter already has quite a few problems with spooky actions otherwise. Supporting something like a double fork flag has very little impact on the complexity of the implementation and even less impact on the efficiency. We certainly are at the point where we can start analyzing the remaining blockers for (v)fork+exec users. > Another example is made obvious by the bug Martin committed a fix for > in the past few days ... the order of operations was incorrect in our > implementation. But that this problem can exist shows that there > are ordering issues - a process that wants to open files using its > current privs, then reset those before (perhaps) opening more files, > and then doing the exec() part cannot do that with posix_spawn(). > Again, this is rare enough that adding the complexity required to allow > it to work just isn't worth it. [In this area also note that the > POSIX_SPAWN_RESETIDS flag causes (effectively) setuid(getuid()), there's > no similar operation to do setuid(geteuid()) ... again too rare to bother.] I quite disagree here, actually. The design-level issue is that POSIX_SPAWN_RESETIDS is a flag and not an action. This means it can't be sequenced and that is the reason for the limitation. There is an obvious parallel with the semantics of the chdir action here--that needs to be that, an action and not just a flag. The separate concern is of course that we need more testing for posix_spawn, but that is hopefully also going to become better as side effect of the non-GSoC project. Joerg
vfork() and posix_spawn() [was: Re: CVS commit: src/lib/libc/sys]
Date:Sat, 12 Jun 2021 23:13:54 +0200
From:Joerg Sonnenberger
Message-ID:
Sorry, missed this message when I was cherry picking messages to read in
a timely fashion.
| On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote:
| > after a vfork() the child can do anything
| This is not true. After vfork(), it is only supposed to call async
| signal safe functions.
What you said, and what I said are not contradictory. Note I said "can
do" and you said "is only supposed to".
What is supposed to work and what actually does work (or can be made to
work) are two different things.
I would note though our man page doesn't make that requirement, or not
that I can see anyway, and vfork() is not in POSIX of course - and while
I don't have a copy to check, I'd suspect not in the C standards either),
so while that sounds like a reasonable requirement to impose for safer
operation, I'm not sure that anyone or anything actually does that.
| That said, a flag for the double fork semantic might be useful.
If the "that said" relates to fork() or vfork() not being async signal safe,
so a double fork() (when the first is vfork anyway) would not be condoned,
that doesn't matter, there to be an async-signal-safe _Fork() call added to
the next version of POSIX, so even with the "only async signal safe"
requirement for vfork() a:
if ((child = vfork()) == 0) {
if (_Fork())_exit(0);
/* do child stuff here */
} else {
if (child != -1) waitpid(child, , 0);
/* parent continues */
}
sequence is possible (will be possible, we don't have _Fork() yet)
and fully conforming, apart from vfork() not being in the standard.
The child of the vfork() only does _Fork() and _exit(), both of which
are permitted. The grandchild process is created by a full fork() so
can do anything at all.
For what it is worth, the definition of _Fork() is (to be):
The _Fork() function shall be equivalent to fork(), except
that fork handlers established by means of the pthread_atfork()
function shall not be called and _Fork() shall be async-signal-safe.
Aside from the prototype line, which is just as you'd expect, that's it.
If (or when) we add it, we probably need to decide whether we need _VFork()
(or some similar name) as well. Probably.
However, an extra posix_spawn attribute to handle double fork is almost
certainly not warranted, this kind of thing isn't all that common, and
adding the mechanism to the posix_spawn set of functions would really
just be bloat (there's also a proposal floating around to add the
ability to change a terminal's process group as a posix_spawn action --
that's required for shells to be able to use posix_spawn in general cases,
but that most likely won't go anywhere).
Remember what the standard says about posix_spawn() (in its rationale, this
is not strictly part of the standard)...
The posix_spawn( ) function and its close relation posix_spawnp( )
have been introduced to overcome the following perceived difficulties
with fork( ): the fork( ) function is difficult or impossible to
implement without swapping or dynamic address translation.
[...]
This view of the role of posix_spawn( ) and posix_spawnp( ) influenced
the design of their API. It does not attempt to provide the full
functionality of fork( )/exec in which arbitrary user-specified
operations of any sort are permitted between the creation of the
child process and the execution of the new process image; any attempt
to reach that level would need to provide a programming language as
parameters.
[...]
[ It lists some of the known problems with the posix_spawn interface ]
[...]
The posix_spawn( ) and posix_spawnp( ) functions do not have all the
power of fork( )/exec. This is to be expected. The fork( ) function
is a wonderfully powerful operation. We do not expect to duplicate
its functionality in a simple, fast function with no special hardware
requirements. It is worth noting that posix_spawn( ) and
posix_spawnp( ) are very similar to the process creation operations
on many operating systems that are not UNIX systems.
It then goes on to list the requirements they had for posix_spawn()...
The requirements for posix_spawn( ) and posix_spawnp( ) are:
� They must be implementable without an MMU or unusual hardware.
� They must be compatible with existing POSIX standards.
Additional goals are:
� They should be efficiently implementable.
� They should be able to replace at least 50% of typical
executions of fork( ).
� A system with posix_spawn( ) and posix_spawnp( ) and without
fork( ) should be useful, at least for realtime applications.
� A
Re: CVS commit: src/lib/libc/sys
On Wed, Jun 09, 2021 at 01:03:20AM +0700, Robert Elz wrote: > It should, when it is workable for the application, make things > faster, while also avoiding the vfork() perils. But only when > it works -- after a vfork() the child can do anything (it should > avoid touching its parent's state, but it can) posix_spawn() isn't > nearly that general, it just handles the most common things apps > are likely to want to do between the fork() and exec(). This is not true. After vfork(), it is only supposed to call async signal safe functions. That said, a flag for the double fork semantic might be useful. Joerg
Re: CVS commit: src/sys/dev
On Wed, 9 Jun 2021, Paul Goyette wrote: Module Name:src Committed By: pgoyette Date: Wed Jun 9 23:22:51 UTC 2021 Modified Files: src/sys/dev: dev_verbose.h Log Message: Use the localcount(9)-based module_hook mechanism to prevent the verbose modules' code and data being unloaded while in use. Should prevent some crashes reported by Riastradh@ to occur during suspend/resume operation. FYI, this commit "rides the bump" introduced a few hours ago by martin@ ++--+-+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com | | Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org | ++--+-+
Re: CVS commit: src/lib/libc/sys
Date:Tue, 8 Jun 2021 16:15:12 +
From:"Nia Alarie"
Message-ID: <20210608161512.1d7c3f...@cvs.netbsd.org>
| vfork.2: recommend posix_spawn instead
You might want to reconsider the wording there, posix_spawn() is
an alternative to [v]fork() + exec*(). Not just vfork().
It should, when it is workable for the application, make things
faster, while also avoiding the vfork() perils. But only when
it works -- after a vfork() the child can do anything (it should
avoid touching its parent's state, but it can) posix_spawn() isn't
nearly that general, it just handles the most common things apps
are likely to want to do between the fork() and exec(). Perhaps
most notably, it has no notion of (with err checking omitted):
if (fork()) { parent_code(/* do a waitpid() first */); }
else if (fork()) { exit(0); }
else { child_code() /* often ending with execX() */ }
when one wants to create an orphan (nothing that cannot be waited upon).
kre
Re: CVS commit: src/sys/dev/audio
On 2021/06/08 16:09, nia wrote: On Tue, Jun 01, 2021 at 09:12:24PM +, Taylor R Campbell wrote: audio(4): Set AUMODE_PLAY/RECORD only if asked _and_ supported. If one is requested and _not_ supported, fail; otherwise we might enter audio_write with a null play track and crash on KASSERT. It looks like this is an incompatible change. Sun says: - Attempts to open a device with FREAD set fail if the device is not capable of recording. (Likewise for FWRITE and playback.) https://github.com/illumos/illumos-gate/blob/9ecd05bdc59e4a1091c51ce68cce2028d5ba6fd1/usr/src/uts/common/io/audio/impl/audio_sun.c#L70 But in NetBSD 7... audio_open() does not return a clear failure: https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L1652 EINVAL is returned if !audio_can_playback in audiostartp() https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L2801 ... which is called from audio_write() So it looks to me like open() should succeed but write() should fail. This is important if you want to open an audio device just to test a few properties (i.e. AUDIO_GETPROPS, AUDIO_GETDEV...). Also, FYI, some tests for audio(4) have started to fail somewhen between audio.c revs 1.96 (this commit) to 1.102: https://releng.netbsd.org/b5reports/i386/commits-2021.06.html#2021.06.02.08.46.16 Thanks, rin
Re: CVS commit: src/sys/dev/audio
On Tue, Jun 01, 2021 at 09:12:24PM +, Taylor R Campbell wrote: > audio(4): Set AUMODE_PLAY/RECORD only if asked _and_ supported. > > If one is requested and _not_ supported, fail; otherwise we might > enter audio_write with a null play track and crash on KASSERT. It looks like this is an incompatible change. Sun says: - Attempts to open a device with FREAD set fail if the device is not capable of recording. (Likewise for FWRITE and playback.) https://github.com/illumos/illumos-gate/blob/9ecd05bdc59e4a1091c51ce68cce2028d5ba6fd1/usr/src/uts/common/io/audio/impl/audio_sun.c#L70 But in NetBSD 7... audio_open() does not return a clear failure: https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L1652 EINVAL is returned if !audio_can_playback in audiostartp() https://github.com/NetBSD/src/blob/netbsd-7/sys/dev/audio.c#L2801 ... which is called from audio_write() So it looks to me like open() should succeed but write() should fail. This is important if you want to open an audio device just to test a few properties (i.e. AUDIO_GETPROPS, AUDIO_GETDEV...).
re: CVS commit: src/tests/lib/csu
Christos Zoulas writes: > In article , Joerg Sonnenberger > wrote: > >On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: > >> In article , Joerg Sonnenberger > > wrote: > >> >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > >> >> Module Name:src > >> >> Committed By: christos > >> >> Date: Thu Jun 3 20:17:37 UTC 2021 > >> >> > >> >> Modified Files: > >> >> src/tests/lib/csu: h_ifunc_static.c > >> >> > >> >> Log Message: > >> >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > >> >support. > >> > > >> >As I mentioned elsewhere, I disagree with this change. OABI is dead and > >> >only really support for legacy compatibility. All the OABI build > >> >variants should be removed and this is strongly a step in the wrong > >> >direction. > >> > >> I think it is better to do it after the 10 branch. It is not just removing > >> the build.sh targets, we need to remove the compat glue and fix the sets, > >> and perhaps something else I am missing. I will revert the two changes > >> once we remove all oabi support. > > > >I don't see how that's related. I'm not talking about removing the > >compat layer right now. Remove the build.sh targets and call it a day. > > CC:ed tech-toolchain. Does everyone agree to remove the oabi build targets? AFAICT, GCC removed the basic oabi targets recently, which is why they weren't updated. we have some dead code in our gcc/arm port in the GCC 10 tree. you can still target oabi with compiler options, but there are no configurations that default to it any more. at this point i'm also wanting to turn off the compat libs for everyone entirely -- if you have old oabi apps, you're welcome to use old libs to run them, same as eg, when we switched from a.out to ELF originally. this is *not* the kernel support to run these apps. ie, remove it from build.sh and share/mk/ ASAP -- before the netbsd 10 branch. .mrg.
Re: CVS commit: src/tests/lib/csu
In article , Joerg Sonnenberger wrote: >On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: >> In article , Joerg Sonnenberger > wrote: >> >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: >> >> Module Name: src >> >> Committed By: christos >> >> Date: Thu Jun 3 20:17:37 UTC 2021 >> >> >> >> Modified Files: >> >> src/tests/lib/csu: h_ifunc_static.c >> >> >> >> Log Message: >> >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc >> >support. >> > >> >As I mentioned elsewhere, I disagree with this change. OABI is dead and >> >only really support for legacy compatibility. All the OABI build >> >variants should be removed and this is strongly a step in the wrong >> >direction. >> >> I think it is better to do it after the 10 branch. It is not just removing >> the build.sh targets, we need to remove the compat glue and fix the sets, >> and perhaps something else I am missing. I will revert the two changes >> once we remove all oabi support. > >I don't see how that's related. I'm not talking about removing the >compat layer right now. Remove the build.sh targets and call it a day. CC:ed tech-toolchain. Does everyone agree to remove the oabi build targets? christos
Re: CVS commit: src/tests/lib/csu
On Fri, Jun 04, 2021 at 10:20:24PM -, Christos Zoulas wrote: > In article , Joerg Sonnenberger > wrote: > >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > >> Module Name: src > >> Committed By: christos > >> Date: Thu Jun 3 20:17:37 UTC 2021 > >> > >> Modified Files: > >>src/tests/lib/csu: h_ifunc_static.c > >> > >> Log Message: > >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > >support. > > > >As I mentioned elsewhere, I disagree with this change. OABI is dead and > >only really support for legacy compatibility. All the OABI build > >variants should be removed and this is strongly a step in the wrong > >direction. > > I think it is better to do it after the 10 branch. It is not just removing > the build.sh targets, we need to remove the compat glue and fix the sets, > and perhaps something else I am missing. I will revert the two changes > once we remove all oabi support. I don't see how that's related. I'm not talking about removing the compat layer right now. Remove the build.sh targets and call it a day. Joerg
Re: CVS commit: src/tests/lib/csu
In article , Joerg Sonnenberger wrote: >On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: >> Module Name: src >> Committed By:christos >> Date:Thu Jun 3 20:17:37 UTC 2021 >> >> Modified Files: >> src/tests/lib/csu: h_ifunc_static.c >> >> Log Message: >> PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc >support. > >As I mentioned elsewhere, I disagree with this change. OABI is dead and >only really support for legacy compatibility. All the OABI build >variants should be removed and this is strongly a step in the wrong >direction. I think it is better to do it after the 10 branch. It is not just removing the build.sh targets, we need to remove the compat glue and fix the sets, and perhaps something else I am missing. I will revert the two changes once we remove all oabi support. christos
Re: CVS commit: src/tests/lib/csu
On Thu, Jun 03, 2021 at 04:17:37PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Thu Jun 3 20:17:37 UTC 2021 > > Modified Files: > src/tests/lib/csu: h_ifunc_static.c > > Log Message: > PR/56230: Jan-Benedict Glaw: arm oabi does not and will not have ifunc > support. As I mentioned elsewhere, I disagree with this change. OABI is dead and only really support for legacy compatibility. All the OABI build variants should be removed and this is strongly a step in the wrong direction. Joerg
Re: CVS commit: src/sys/kern
Hmm, misleading commit log (code itself is correct). More precisely:
s/the last element of ksyms_symtabs/ksyms_last_snapshot/
Anyway, does this help you to reintroduce
"ksyms(4): Don't skip symbol tables that are soon to be freed."?
That KASSERT does not fire anymore for aarch64, as far as I can see.
Thanks,
rin
On 2021/06/03 0:43, Rin Okuyama wrote:
Module Name:src
Committed By: rin
Date: Wed Jun 2 15:43:33 UTC 2021
Modified Files:
src/sys/kern: kern_ksyms.c
Log Message:
Fix regression introduced in rev 1.90:
http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/kern/kern_ksyms.c#rev1.90
in which the last element of ksyms_symtabs is skipped by mistake.
To generate a diff of this commit:
cvs rdiff -u -r1.93 -r1.94 src/sys/kern/kern_ksyms.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/kern/kern_ksyms.c
diff -u src/sys/kern/kern_ksyms.c:1.93 src/sys/kern/kern_ksyms.c:1.94
--- src/sys/kern/kern_ksyms.c:1.93 Wed Jun 2 08:46:16 2021
+++ src/sys/kern/kern_ksyms.c Wed Jun 2 15:43:33 2021
@@ -1,4 +1,4 @@
-/* $NetBSD: kern_ksyms.c,v 1.93 2021/06/02 08:46:16 riastradh Exp $
*/
+/* $NetBSD: kern_ksyms.c,v 1.94 2021/06/02 15:43:33 rin Exp $ */
/*-
* Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -73,7 +73,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.93 2021/06/02 08:46:16 riastradh Exp
$");
+__KERNEL_RCSID(0, "$NetBSD: kern_ksyms.c,v 1.94 2021/06/02 15:43:33 rin Exp
$");
#if defined(_KERNEL) && defined(_KERNEL_OPT)
#include "opt_copy_symtab.h"
@@ -1087,7 +1087,7 @@ ksymsread(dev_t dev, struct uio *uio, in
*/
filepos = sizeof(struct ksyms_hdr);
for (st = TAILQ_FIRST(_symtabs);
-st != ksyms_last_snapshot;
+st != TAILQ_NEXT(ksyms_last_snapshot, sd_queue);
st = TAILQ_NEXT(st, sd_queue)) {
if (__predict_false(st->sd_gone))
continue;
@@ -1109,7 +1109,7 @@ ksymsread(dev_t dev, struct uio *uio, in
KASSERT(filepos <= sizeof(struct ksyms_hdr) +
ksyms_hdr.kh_shdr[SYMTAB].sh_size);
for (st = TAILQ_FIRST(_symtabs);
-st != ksyms_last_snapshot;
+st != TAILQ_NEXT(ksyms_last_snapshot, sd_queue);
st = TAILQ_NEXT(st, sd_queue)) {
if (__predict_false(st->sd_gone))
continue;
Re: CVS commit: src/sys/kern
> Date: Wed, 2 Jun 2021 17:33:39 +0900 > From: Rin Okuyama > > On 2021/06/02 6:11, Taylor R Campbell wrote: > > - KASSERT(filepos <= sizeof(struct ksyms_hdr) + > > + KASSERT(filepos == sizeof(struct ksyms_hdr) + > > ksyms_hdr.kh_shdr[SYMTAB].sh_size); > ... > > This KASSERT fires at least for arm and aarch64, with savecore or ``nm > /dev/ksyms'': Oops. I reverted that change for now, will investigate later.
Re: CVS commit: src/sys/kern
Hi, On 2021/06/02 6:11, Taylor R Campbell wrote: Module Name:src Committed By: riastradh Date: Tue Jun 1 21:11:52 UTC 2021 Modified Files: src/sys/kern: kern_ksyms.c Log Message: ksyms(4): Don't skip symbol tables that are soon to be freed. They will not actually be freed until /dev/ksyms is closed, so continued access to them remains kosher. To generate a diff of this commit: cvs rdiff -u -r1.91 -r1.92 src/sys/kern/kern_ksyms.c ... Index: src/sys/kern/kern_ksyms.c diff -u src/sys/kern/kern_ksyms.c:1.91 src/sys/kern/kern_ksyms.c:1.92 --- src/sys/kern/kern_ksyms.c:1.91 Tue Jun 1 21:11:07 2021 +++ src/sys/kern/kern_ksyms.c Tue Jun 1 21:11:52 2021 ... - KASSERT(filepos <= sizeof(struct ksyms_hdr) + + KASSERT(filepos == sizeof(struct ksyms_hdr) + ksyms_hdr.kh_shdr[SYMTAB].sh_size); ... This KASSERT fires at least for arm and aarch64, with savecore or ``nm /dev/ksyms'': $ nm /dev/ksyms [ 1501.3538912] panic: kernel diagnostic assertion "filepos == sizeof(struct ksyms_hdr) + ksyms_hdr.kh_shdr[SYMTAB].sh_size" failed: file "../../../../kern/kern_ksyms.c", line 1107 [ 1501.3701109] cpu0: Begin traceback... [ 1501.3701109] 0xc9c2ecfc: netbsd:db_panic+0xc [ 1501.3806387] 0xc9c2ed14: netbsd:vpanic+0xc4 [ 1501.3806387] 0xc9c2ed2c: netbsd:kern_assert+0x3c [ 1501.3906766] 0xc9c2ed6c: netbsd:ksymsread+0x208 [ 1501.4006964] 0xc9c2ee1c: netbsd:spec_read+0xbc [ 1501.4112302] 0xc9c2ee44: netbsd:VOP_READ+0x58 [ 1501.4112302] 0xc9c2ee6c: netbsd:vn_read+0x78 [ 1501.4215160] 0xc9c2eebc: netbsd:dofileread+0x80 [ 1501.4315439] 0xc9c2eeec: netbsd:sys_read+0x50 [ 1501.4426847] 0xc9c2efac: netbsd:syscall+0x188 [ 1501.4426847] cpu0: End traceback... Stopped in pid 11825.11825 (nm) at netbsd:cpu_Debugger+0x4:bx r14 db> Can you please take a look? Thanks, rin
Re: CVS commit: src/sys/lib/libunwind
Correction: On 2021/06/01 7:10, Rin Okuyama wrote: On 2021/05/31 23:35, Joerg Sonnenberger wrote: On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote: Bump LAST_REGISTER and LAST_RESTORE_REG to REGNO_ARM32_S31 for arm. This is not desirable as it significantly increases the memory use. The goal here is to actually alias the single and double register in the space. That's what the existing was doing. Existing code did not. With LAST_REGISTER = REGNO_ARM32_D31, libunwind gave up unwinding frames when it encounters s0-s31; see parseInstructions(): https://nxr.netbsd.org/xref/src/sys/lib/libunwind/DwarfParser.hpp#parseInstructions This paragraph: And with LAST_RESTORE_REG = REGNO_ARM32_D31, it did not copy-back contents of s0-s31 at all; see stepWithDwarf(): https://nxr.netbsd.org/xref/src/sys/lib/libunwind/DwarfInstructions.hpp#136 is only applied to the case of LAST_REGISTER = REGNO_ARM32_S31, and not to existing code. However, LAST_RESTORE_REG itself has nothing to do with memory usage. And moreover, the conclusion below is not changed at all. Thanks, rin IMO, in order to support VFP-register aliasing, we need to heavily modify MI codes. This adds complexities to the code, and only ARM would receive benefits from them. Existing code was not acceptable for me; for example, GDB aborted *every time* exceptions were raised. I won't revert this change at the moment. Please provide working alternative if you don't like mine. Thanks, rin Joerg
Re: CVS commit: src/sys/lib/libunwind
On 2021/05/31 23:35, Joerg Sonnenberger wrote: On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote: Bump LAST_REGISTER and LAST_RESTORE_REG to REGNO_ARM32_S31 for arm. This is not desirable as it significantly increases the memory use. The goal here is to actually alias the single and double register in the space. That's what the existing was doing. Existing code did not. With LAST_REGISTER = REGNO_ARM32_D31, libunwind gave up unwinding frames when it encounters s0-s31; see parseInstructions(): https://nxr.netbsd.org/xref/src/sys/lib/libunwind/DwarfParser.hpp#parseInstructions And with LAST_RESTORE_REG = REGNO_ARM32_D31, it did not copy-back contents of s0-s31 at all; see stepWithDwarf(): https://nxr.netbsd.org/xref/src/sys/lib/libunwind/DwarfInstructions.hpp#136 IMO, in order to support VFP-register aliasing, we need to heavily modify MI codes. This adds complexities to the code, and only ARM would receive benefits from them. Existing code was not acceptable for me; for example, GDB aborted *every time* exceptions were raised. I won't revert this change at the moment. Please provide working alternative if you don't like mine. Thanks, rin Joerg
Re: CVS commit: src/sys/lib/libunwind
On 2021/05/31 23:32, Joerg Sonnenberger wrote: On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote: There are two numbering schemes for VFPv2 registers: s0-s31 and d0-d15. The former is used by GCC, and the latter is by LLVM. Since libunwind was derived from LLVM, it has never supported the former. This results in crashes for GCC-compiled binaries in exception handler of C++, if it encounters VFPv2 registers when unwinding frames. This is only half correct. GCC actually switched at some point. I don't think so. At least, when they supported VFPv3 back to 2009: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=854b8a40570fee8d8c22acf21355a8ab88f17557 and when they introduced arm_dbx_register_number(), which converts b/w internal/DWARF regnum, back to 2005: https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=2fa330b29a650365d4d88e4407fdbc2934dcb1b4 they used s0-s31 numbering for d0-d15. More accurately, I *imagine*, GCC actually switched at some point to use VFP registers for general purposes (mainly as cache for stack variables for -O2, as far as I can see). Before this switch, only applications using floating-point arithmetic were affected. But, after that, most C++ applications became affected, and the problem got visible to us. Thanks, rin Joerg
Re: CVS commit: src/sys/lib/libunwind
On 2021/05/31 23:30, Joerg Sonnenberger wrote: On Mon, May 31, 2021 at 11:41:22AM +, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Mon May 31 11:41:22 UTC 2021 Modified Files: src/sys/lib/libunwind: Registers.hpp unwind_registers.S Log Message: ... - Introduce enum for flags. Please undo this part. enums should *not* be used for flags. Done. Thanks for pointing it out. rin Joerg
Re: CVS commit: src/sys/lib/libunwind
On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote: > Bump LAST_REGISTER and LAST_RESTORE_REG to REGNO_ARM32_S31 for arm. This is not desirable as it significantly increases the memory use. The goal here is to actually alias the single and double register in the space. That's what the existing was doing. Joerg
Re: CVS commit: src/sys/lib/libunwind
On Mon, May 31, 2021 at 12:12:24PM +, Rin Okuyama wrote: > There are two numbering schemes for VFPv2 registers: s0-s31 and d0-d15. > The former is used by GCC, and the latter is by LLVM. Since libunwind was > derived from LLVM, it has never supported the former. This results in > crashes for GCC-compiled binaries in exception handler of C++, if it > encounters VFPv2 registers when unwinding frames. This is only half correct. GCC actually switched at some point. Joerg
Re: CVS commit: src/sys/lib/libunwind
On Mon, May 31, 2021 at 11:41:22AM +, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Mon May 31 11:41:22 UTC 2021 > > Modified Files: > src/sys/lib/libunwind: Registers.hpp unwind_registers.S > > Log Message: > ... > > - Introduce enum for flags. Please undo this part. enums should *not* be used for flags. Joerg
Re: CVS commit: src/sys/arch/arm/include/arm32
"Rin Okuyama" wrote: > Module Name: src > Committed By: rin > Date: Sun May 30 07:20:00 UTC 2021 > > Modified Files: > > src/sys/arch/arm/include/arm32: param.h > > Log Message: > > Include opt_param.h for MSGBUFSIZE ifdef _KERNEL_OPT. Thanks Rin! I thought I had checked all the ways MSGBUFSIZE was pulled in. Cheers, Simon.
Re: CVS commit: src/share/mk
Ok, I did not realize that all mips had switched. christos > On May 27, 2021, at 4:48 PM, matthew green wrote: > > "Christos Zoulas" writes: >> Module Name: src >> Committed By:christos >> Date:Thu May 27 20:29:24 UTC 2021 >> >> Modified Files: >> src/share/mk: bsd.own.mk >> >> Log Message: >> mips64 only works with gcc-10 > > the MACHINE_CPU==mips test should cover mipsn64, that i assume > you mean here... > > > .mrg. signature.asc Description: Message signed with OpenPGP
re: CVS commit: src/share/mk
"Christos Zoulas" writes: > Module Name: src > Committed By: christos > Date: Thu May 27 20:29:24 UTC 2021 > > Modified Files: > src/share/mk: bsd.own.mk > > Log Message: > mips64 only works with gcc-10 the MACHINE_CPU==mips test should cover mipsn64, that i assume you mean here... .mrg.
Re: CVS commit: src/distrib/acorn32/stand
> On May 24, 2021, at 9:02 PM, matthew green wrote: > > this also probably fails to work as wanted (while not > actually failing) for people who use "cvs -D .CVS", etc. > > it's kinda gross. It is gross, and I considered removing it. christos signature.asc Description: Message signed with OpenPGP
re: CVS commit: src/distrib/acorn32/stand
"Christos Zoulas" writes: > Module Name: src > Committed By: christos > Date: Mon May 24 22:40:44 UTC 2021 > > Modified Files: > src/distrib/acorn32/stand: Makefile > > Log Message: > PR/56207: Jan-Benedict Glaw: Handle error from find when removing CVS > directories on a git repo. this also probably fails to work as wanted (while not actually failing) for people who use "cvs -D .CVS", etc. it's kinda gross.
Re: CVS commit: src
On Mon, May 17, 2021 at 01:12:12PM -0400, Christos Zoulas wrote: > Module Name: src > Committed By: christos > Date: Mon May 17 17:12:12 UTC 2021 > > Modified Files: > src: build.sh > > Log Message: > for mercurial, use the latest revision instead of limiting the output to 1 > (requested by joerg) Just for the record, this uses the currently checked out revision and not the latest revision. "hg log" willout a revset starts with whatever was committed/pulled last. Joerg
Re: CVS commit: src/tools/gcc
I understand. Thanks! rin On 2021/05/09 9:20, Christos Zoulas wrote: In article , Rin Okuyama wrote: How about binutils? http://cvsweb.netbsd.org/bsdweb.cgi/src/tools/binutils/Makefile#rev1.32 I don't think it makes a difference. It seems that we need to mknative again for this: Eventually, but not right now. christos
Re: CVS commit: src/tools/gcc
In article , Rin Okuyama wrote: >How about binutils? > >http://cvsweb.netbsd.org/bsdweb.cgi/src/tools/binutils/Makefile#rev1.32 I don't think it makes a difference. >It seems that we need to mknative again for this: Eventually, but not right now. christos
Re: CVS commit: src/tools/gcc
How about binutils? http://cvsweb.netbsd.org/bsdweb.cgi/src/tools/binutils/Makefile#rev1.32 It seems that we need to mknative again for this: % find /usr/src/external/gpl3/binutils/usr.bin/ld -name '*.h' | xargs grep HAVE_INITFINI_ARRAY /usr/src/external/gpl3/binutils/usr.bin/ld/arch/aarch64/config.h:/* #undef HAVE_INITFINI_ARRAY */ /usr/src/external/gpl3/binutils/usr.bin/ld/arch/aarch64eb/config.h:/* #undef HAVE_INITFINI_ARRAY */ ...(snip)... Thanks, rin On 2021/05/09 4:36, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sat May 8 19:36:28 UTC 2021 Modified Files: src/tools/gcc: Makefile Log Message: Disable again initfini; breaks some archs and not worth dealing with when we have both gcc's active in the tree. To generate a diff of this commit: cvs rdiff -u -r1.103 -r1.104 src/tools/gcc/Makefile Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/net
In article <20210506061816.94bb1f...@cvs.netbsd.org>, Shoichi YAMAGUCHI wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: yamaguchi >Date: Thu May 6 06:18:16 UTC 2021 > >Modified Files: > src/sys/net: if_spppsubr.c > >Log Message: >do not clear destination address if there is no saved address >and add initialization of saved_hisaddr for safety You don't need to byte-swap in order to compare with 0... :-) christos
Re: CVS commit: src/external/gpl3/binutils/dist/bfd
On 2021/04/22 10:09, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Thu Apr 22 01:09:48 UTC 2021 Modified Files: src/external/gpl3/binutils/dist/bfd: elf32-ppc.c elf64-ppc.c Log Message: Fix regression where ld(1) is trapped into infinite loop when linking binary whose text does not fit within R_PPC_REL24. Reported upstream as Bug 27755: https://sourceware.org/bugzilla/show_bug.cgi?id=27755 This problem was introduced to binutils-2-31-1 for our tree. netbsd-9 is affected, while netbsd-8 is not. For elf64-ppc.c, the same fix had already been applied to master branch, which I overlooked: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=435edf0bf231240ccecb474b74ebb49dc8db2633 For elf32-ppc.c, the fix was just committed: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=337d0bf887a3ed6b6b2123fecfec0736640edcaf I'll send pullup request to netbsd-9. Thanks, rin
Re: CVS commit: src/external/bsd/compiler_rt/lib/clang/lib/netbsd
On 30.04.2021 15:42, Tobias Nygren wrote: On Tue, 20 Apr 2021 23:19:53 + Roland Illig wrote: Module Name:src Committed By: rillig Date: Tue Apr 20 23:19:53 UTC 2021 Modified Files: src/external/bsd/compiler_rt/lib/clang/lib/netbsd: syms.mk Log Message: clang: fix build for installing libclang_rt.ubsan_minimal-x86_64.a Hi! My HEAD-llvm build (with empty obj and tool dir) fails since this change, with the below output. reverting to syms.mk,v 1.3 gets me a working tree. Thanks for the notice. I fixed this in the individual Makefiles of the subdirectories (revision 1.5). Roland
Re: CVS commit: src/include
Hmm, it seems that we were the only ones that had RES_NOCHECKNAME enabled. Everyone else uses check-names by default. So I am inclined to leave it as is. christos > On Apr 30, 2021, at 5:33 PM, Christos Zoulas wrote: > > Signed PGP part > I guess I will undo it, because as I mentioned in a separate mail it causes > other problems. > > christos > >> On Apr 30, 2021, at 5:18 PM, Robert Elz wrote: >> >> Date:Fri, 30 Apr 2021 12:07:49 -0400 >> From:"Christos Zoulas" >> Message-ID: <20210430160749.3a4dbf...@cvs.netbsd.org> >> >> |src/include: resolv.h >> | >> | Log Message: >> | Default to check-names for safety. >> >> Please don't do that, check-names applies at the wrong place, and >> is far too big a hammer. Applications which actually depend upon >> names returned (all that ever matters) having (or not having) some >> particular syntax should be verifying that before using it, just like >> any other data received over the network. What is to be valid, and >> what is a problem, varies from application to application. >> >> kre > > > signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/include
I guess I will undo it, because as I mentioned in a separate mail it causes other problems. christos > On Apr 30, 2021, at 5:18 PM, Robert Elz wrote: > >Date:Fri, 30 Apr 2021 12:07:49 -0400 >From:"Christos Zoulas" >Message-ID: <20210430160749.3a4dbf...@cvs.netbsd.org> > > |src/include: resolv.h > | > | Log Message: > | Default to check-names for safety. > > Please don't do that, check-names applies at the wrong place, and > is far too big a hammer. Applications which actually depend upon > names returned (all that ever matters) having (or not having) some > particular syntax should be verifying that before using it, just like > any other data received over the network. What is to be valid, and > what is a problem, varies from application to application. > > kre signature.asc Description: Message signed with OpenPGP
Re: CVS commit: src/include
Date:Fri, 30 Apr 2021 12:07:49 -0400 From:"Christos Zoulas" Message-ID: <20210430160749.3a4dbf...@cvs.netbsd.org> | src/include: resolv.h | | Log Message: | Default to check-names for safety. Please don't do that, check-names applies at the wrong place, and is far too big a hammer. Applications which actually depend upon names returned (all that ever matters) having (or not having) some particular syntax should be verifying that before using it, just like any other data received over the network. What is to be valid, and what is a problem, varies from application to application. kre
Re: CVS commit: src/external/bsd/compiler_rt/lib/clang/lib/netbsd
On Tue, 20 Apr 2021 23:19:53 +
Roland Illig wrote:
> Module Name: src
> Committed By: rillig
> Date: Tue Apr 20 23:19:53 UTC 2021
>
> Modified Files:
> src/external/bsd/compiler_rt/lib/clang/lib/netbsd: syms.mk
>
> Log Message:
> clang: fix build for installing libclang_rt.ubsan_minimal-x86_64.a
Hi!
My HEAD-llvm build (with empty obj and tool dir) fails since this
change, with the below output.
reverting to syms.mk,v 1.3 gets me a working tree.
install ===> external/bsd/compiler_rt/lib/clang/lib/netbsd/asan-m64
nbmake[8]: don't know how to make libclang_rt.asan-x86_64.a.syms. Stop
nbmake[8]: stopped in
/work/src/external/bsd/compiler_rt/lib/clang/lib/netbsd/asan-m64
*** Failed target: install-asan-m64
mk.conf settings used:
MKLLVM=yes
MKGCC=no
HAVE_LLVM=yes
MKCOMPAT=no
MAKEVERBOSE=1
MKATF=no
MKHTML=no
MKINFO=no
MKIPFILTER=no
MKKYUA=no
MKPF=no
MKPIGZGZIP=yes
MKPROFILE=no
MKLVM=no
MKLDAP=no
MKMDNS=no
MKRUMP=no
MKDTRACE=no
MKLLVMRT=no
MKUNBOUND=no
MKKMOD=no
Also have this local patch.
--- external/gpl3/gdb/dist/gdb/unittests/string_view-selftests.c15 Sep
2020 01:43:48 - 1.1.1.2
+++ external/gpl3/gdb/dist/gdb/unittests/string_view-selftests.c30 Apr
2021 13:38:46 -
@@ -177,6 +177,5 @@ void
_initialize_string_view_selftests ()
{
#if defined(GDB_STRING_VIEW)
- selftests::register_test ("string_view", selftests::string_view::run_tests);
#endif
}
Re: CVS commit: src/sys
> Module Name:src > Committed By: thorpej > Date: Sat Apr 24 23:37:01 UTC 2021 > > Log Message: > Merge thorpej-cfargs branch: > > Simplify and make extensible the config_search() / config_found() / > config_attach() interfaces: rather than having different variants for > which arguments you want pass along, just have a single call that > takes a variadic list of tag-value arguments. I hate to do this for such a large merge, and I appreciate what you've done to work on fixing fallout promptly, but... I have to ask that this be reverted. I didn't realize that your message to tech-kern meant `I am going to merge this imminently', and although my response on tech-kern didn't go out until after the merge (I'm behind on source-changes, didn't notice you'd already merged a few hours before I replied), I had registered the same objections privately before the merge: This creates a new class of stack garbage bugs that the compiler will do nothing to help detect (mismatched tag/value pairs, missing EOL), and, worse, it affects many code paths that are essentially guaranteed never to be exercised by automatic tests because they are all scattered throughout device drivers for hardware which is mostly not virtualized. Even if the config_found_ia_ia_cthulhu_fhtagn API may have too many variant functions, at least they have types the compiler can check (aside from the attach args passed as void * -- which is a design mistake we should fix!). I'm still a little unclear on what the benefits of the change are, but losing type safety here is a serious regression. We can discuss better ways to do the config_found* API, but that discussion needs to happen first (and concerns addressed, not brushed off) before investment in widespread changes. Let's please take advantage of C's type system to avoid creating new bug classes, especially new bug classes that can't be automatically tested -- I suggested a couple of ways to do that already, and I think we could do even better with some tweaks to config(8) to enable typed interface attributes.
re: CVS commit: src/sys/net
> (I haven't investigated exactly what's going on leading to ~5 KB-byte > stack frames, but this shuts gcc up, at least, and the hypotheses > sound plausible to me!) this matches what i saw when looking at it. i was going to test the exact same idea (noinline). .mrg.
Re: CVS commit: src/external/gpl3/gcc/usr.bin/cc1plus
The condition is reversed. I will fix.
christos
> On Apr 26, 2021, at 10:31 PM, Rin Okuyama wrote:
>
> Hi,
>
> On 2021/04/26 7:25, Christos Zoulas wrote:
>> --- src/external/gpl3/gcc/usr.bin/cc1plus/Makefile:1.15 Sun Apr 11
>> 20:05:56 2021
>> +++ src/external/gpl3/gcc/usr.bin/cc1plus/Makefile Sun Apr 25 18:25:55 2021
> (snip)
>> -.if ${MACHINE_ARCH} == "mipseb" || ${MACHINE_ARCH} == "mipsel"
>> +.if ${MACHINE_MIPS64}
>> COPTS.c-common.c+=-O3
>> .endif
>
> This was hack for mips32:
>
> http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/usr.bin/cc1plus/Makefile#rev1.12
>
> Does mips64 require it? Also for
> external/gpl3/gcc/usr.bin/cc1objplus/Makefile.
>
> Thanks,
> rin
signature.asc
Description: Message signed with OpenPGP
Re: CVS commit: src/external/gpl3/gcc/usr.bin/cc1plus
Hi,
On 2021/04/26 7:25, Christos Zoulas wrote:
--- src/external/gpl3/gcc/usr.bin/cc1plus/Makefile:1.15 Sun Apr 11 20:05:56 2021
+++ src/external/gpl3/gcc/usr.bin/cc1plus/Makefile Sun Apr 25 18:25:55 2021
(snip)
-.if ${MACHINE_ARCH} == "mipseb" || ${MACHINE_ARCH} == "mipsel"
+.if ${MACHINE_MIPS64}
COPTS.c-common.c+=-O3
.endif
This was hack for mips32:
http://cvsweb.netbsd.org/bsdweb.cgi/src/external/gpl3/gcc/usr.bin/cc1plus/Makefile#rev1.12
Does mips64 require it? Also for external/gpl3/gcc/usr.bin/cc1objplus/Makefile.
Thanks,
rin
Re: CVS commit: src/sys/dev
On Sat, 24 Apr 2021, Robert Elz wrote:
Date:Sat, 24 Apr 2021 00:15:37 +
From:"Michael Lorenz"
Message-ID: <20210424001537.c5c83f...@cvs.netbsd.org>
| add an ioctl() to get a list of fonts currently available via wsfont
It seems to me it would be useful for that ioctl to copyout()
the fi_numentries field of the struct (if addr != NULL) from
wsdisplayio_listfonts() just before the ENOMEM check (so it is
updated, even if ENOMEM is returned). (Does it make any sense
for addr to be NULL, or should that be an error? EINVAL or something.)
Otherwise, there doesn't seem to be any easy way for the user of
the ioctl to know how many fonts were returned (checking which elements
of the array were modified is not "easy") or how big the buffer would
need to be to fetch all of them in the ENOMEM case.
In several other places, we return "total space needed" separately,
regardless of how much data was actually copied. The general paradigm
is (more or less)
buff = NULL;
size = 0;
err = func(..., buff, size, );
while (err == 0) {
if (need > size) {
free(buff);
buff = malloc(need);
if (buff == NULL)
err = ENOMEM;
}
}
For a real-life example look at the modctl(2) code for MODULE_STAT
++--+---+
| Paul Goyette | PGP Key fingerprint: | E-mail addresses: |
| (Retired) | FA29 0E3B 35AF E8AE 6651 | p...@whooppee.com |
| Software Developer | 0786 F758 55DE 53BA 7731 | pgoye...@netbsd.org |
++--+---+
Re: CVS commit: src/sys/dev
Oh, I see from your code change to wsfontload.c that you intended for the fi_numentries field to get copied out, it just doesn't seem to happen. I also see that the addr==NULL case happens if malloc() (in wsfontload.c) failed - going ahead with the ioctl() in that case seems like a mistake, optimising away the error checking that way looks fragile. (And the magic 4096 even moreso). Just check the malloc() result, and then in the ioctl code, test for it as well, and return an error from there, rather than doing all the work for no benefit in that case.Alternatively, you could define that case to be a "fetch the count" variant of the ioctl, where all that happens in that case is that the fi_numentries field of the struct is filled in and returned. kre kre
Re: CVS commit: src/sys/dev
Date:Sat, 24 Apr 2021 00:15:37 + From:"Michael Lorenz" Message-ID: <20210424001537.c5c83f...@cvs.netbsd.org> | add an ioctl() to get a list of fonts currently available via wsfont It seems to me it would be useful for that ioctl to copyout() the fi_numentries field of the struct (if addr != NULL) from wsdisplayio_listfonts() just before the ENOMEM check (so it is updated, even if ENOMEM is returned). (Does it make any sense for addr to be NULL, or should that be an error? EINVAL or something.) Otherwise, there doesn't seem to be any easy way for the user of the ioctl to know how many fonts were returned (checking which elements of the array were modified is not "easy") or how big the buffer would need to be to fetch all of them in the ENOMEM case. It might also be worth allowing fi_numentries to also be an input parameter, to indicate where in the set of fonts the fetch should start, to provide a mechanism to cope should the list size ever grow so big that it ends up bigger than a single ioctl can handle (that is, skip the first fi_numentries fonts, and then continue from there). That would mean a slightly more complex piece of internal code though. And this last bit is just style, but I'd change the struct wsdisplayio_fontdesc to be uint32_t fd_len; uint16_t fd_height; unit16_t fd_width; char fd_name[]; and then make the entries returned be (rounded up) just big enough for the actual font names. But that's just me, and not important. kre
re: CVS commit: src/distrib/sets/lists/debug
"Rin Okuyama" writes: > Module Name: src > Committed By: rin > Date: Fri Apr 23 15:21:49 UTC 2021 > > Modified Files: > src/distrib/sets/lists/debug: mi > > Log Message: > Add lto-dump.debug. thanks! i had changed this, but failed to commit. .mrg.
Re: CVS commit: src/sys/arch/powerpc/include/booke
On 2021/04/18 0:38, Joerg Sonnenberger wrote: On Sat, Apr 17, 2021 at 01:25:57PM +, Rin Okuyama wrote: Module Name:src Committed By: rin Date: Sat Apr 17 13:25:57 UTC 2021 Modified Files: src/sys/arch/powerpc/include/booke: vmparam.h Log Message: Sync MAXfoo params with oea: MAXTSIZ: 512MB -> 128MB MAXDSIZ: 3.25GB -> 1GB There should be no particular reasons for having different values. Is there an architectural reason for MAXTSIZ to be 128MB? Because we have seen binaries larger than that. Ah, I just forgot recent discussion on MAXTSIZ. There's no limitation due to architecture. I've confirmed binary just works fine, whose text is larger than 256MB segment on oea. It works on booke and ibm4xx also. Maybe we can leave MAXTSIZ undefined for powerpc. Thanks, rin
Re: CVS commit: src/usr.bin/xlint/lint1
On 18.04.2021 16:12, Christos Zoulas wrote: In article <20210418085205.10b75f...@cvs.netbsd.org>, Roland Illig wrote: -=-=-=-=-=- Module Name:src Committed By: rillig Date: Sun Apr 18 08:52:04 UTC 2021 Modified Files: src/usr.bin/xlint/lint1: err.c externs1.h lint1.h Log Message: lint: add error_at, warning_at, message_at Isn't it better to pass "const pos_t *", instead of pos_t which needs to copy the struct? Thanks for the suggestion, I just changed that. Roland
Re: CVS commit: src/usr.bin/xlint/lint1
In article <20210418085205.10b75f...@cvs.netbsd.org>, Roland Illig wrote: >-=-=-=-=-=- > >Module Name: src >Committed By: rillig >Date: Sun Apr 18 08:52:04 UTC 2021 > >Modified Files: > src/usr.bin/xlint/lint1: err.c externs1.h lint1.h > >Log Message: >lint: add error_at, warning_at, message_at > >Right now there are several places in the code that use the global >variable curr_pos for passing a parameter to the diagnostic functions. >That's not what global variables are for. > >Make it easy for the code to migrate to the parameter-passing style. > Isn't it better to pass "const pos_t *", instead of pos_t which needs to copy the struct? christos
Re: CVS commit: src/sys/arch/powerpc/include/booke
On Sat, Apr 17, 2021 at 01:25:57PM +, Rin Okuyama wrote: > Module Name: src > Committed By: rin > Date: Sat Apr 17 13:25:57 UTC 2021 > > Modified Files: > src/sys/arch/powerpc/include/booke: vmparam.h > > Log Message: > Sync MAXfoo params with oea: > > MAXTSIZ: 512MB -> 128MB > MAXDSIZ: 3.25GB -> 1GB > > There should be no particular reasons for having different values. Is there an architectural reason for MAXTSIZ to be 128MB? Because we have seen binaries larger than that. Joerg
Re: CVS commit: src/external/bsd/atf/dist/tools
"Andreas Gustafsson" wrote: > Module Name: src > Committed By: gson > Date: Sat Apr 10 10:32:57 UTC 2021 > > Modified Files: > > src/external/bsd/atf/dist/tools: atf-run.1 atf-run.cpp > > Log Message: > > Add support for running individual test cases under isolation. Thank you! This will be useful to debug individual test case failures. Cheers, Simon.
re: CVS commit: src/crypto/external/bsd/openssl/lib/libcrypto/arch/sparc
"Martin Husemann" writes: > Module Name: src > Committed By: martin > Date: Thu Apr 8 15:06:50 UTC 2021 > > Removed Files: > src/crypto/external/bsd/openssl/lib/libcrypto/arch/sparc: modes.inc > > Log Message: > Do not pretend we have GHASH asm code please see my other message -- this is relevant for sparc64 32 bit builds as well as sparc builds. the fix christos made to restore __arch64__ should be used similarly here, i guess. .mrg.
Re: CVS commit: src/lib
joerg> Just the libs should be enough and ideally, jk> It appears that we would need to add the 'elftoolchain' subdirectory to jk> "src/external/bsd/Makefile" for the global includes run to work. This should be fixed now. Regards, Joseph Koshy
Re: CVS commit: src/lib
On Thu, Apr 08, 2021 at 05:53:42PM +0100, Joseph Koshy wrote: > On Thu, Apr 08, 2021 at 01:51:30PM +0200, Joerg Sonnenberger wrote: > jk> Redo r1.288: traverse the complete imported Elftoolchain tree during > jk> a build. > > js> Just the libs should be enough and ideally, libelf and libdwarf > js> are folded into the main .WAIT groups. There is a global includes > js> run first. > > It appears that we would need to add the 'elftoolchain' subdirectory to > "src/external/bsd/Makefile" for the global includes run to work. > > Let me try that out - once I get it to work I will revert > 'src/lib/Makefile' to traverse just the imported libraries. Thanks! Joerg
Re: CVS commit: src/lib
On Thu, Apr 08, 2021 at 01:51:30PM +0200, Joerg Sonnenberger wrote: jk> Redo r1.288: traverse the complete imported Elftoolchain tree during jk> a build. js> Just the libs should be enough and ideally, libelf and libdwarf js> are folded into the main .WAIT groups. There is a global includes js> run first. It appears that we would need to add the 'elftoolchain' subdirectory to "src/external/bsd/Makefile" for the global includes run to work. Let me try that out - once I get it to work I will revert 'src/lib/Makefile' to traverse just the imported libraries. Regards, Joseph Koshy
Re: CVS commit: src/lib
On Thu, Apr 08, 2021 at 08:10:30AM +, Joseph Koshy wrote: > Module Name: src > Committed By: jkoshy > Date: Thu Apr 8 08:10:30 UTC 2021 > > Modified Files: > src/lib: Makefile > > Log Message: > Redo r1.288: traverse the complete imported Elftoolchain tree during a build. Just the libs should be enough and ideally, libelf and libdwarf are folded into the main .WAIT groups. There is a global includes run first. Joerg
Re: CVS commit: src
Hi, Thanks for your quick fix! I will rebuild userland and ruby30-base. On Wed, Apr 7, 2021 at 7:00 PM Simon Burge wrote: > > Ryo ONODERA wrote: > > > Hi, > > > > dtrace support of pkgsrc/lang/ruby30-base uses drti.o. > > Without drti.o, ruby30-base is not buildable with dtrace option > > and dtrace option is enabled by default. > > > > Could you please put drti.o back? > > Thanks for the bug report. I've reverted this change. > > Background - I was having problems getting the MIPS dtrace build working > with all available ABIs and couldn't find any use of drti.o anywhere in > the tree. It didn't occur to me that it might be used (actively!) by > packages. > > Cheers, > Simon.
Re: CVS commit: src
Ryo ONODERA wrote: > Hi, > > dtrace support of pkgsrc/lang/ruby30-base uses drti.o. > Without drti.o, ruby30-base is not buildable with dtrace option > and dtrace option is enabled by default. > > Could you please put drti.o back? Thanks for the bug report. I've reverted this change. Background - I was having problems getting the MIPS dtrace build working with all available ABIs and couldn't find any use of drti.o anywhere in the tree. It didn't occur to me that it might be used (actively!) by packages. Cheers, Simon.
Re: CVS commit: src
Hi, dtrace support of pkgsrc/lang/ruby30-base uses drti.o. Without drti.o, ruby30-base is not buildable with dtrace option and dtrace option is enabled by default. Could you please put drti.o back? Thank you. On Mon, Mar 29, 2021 at 10:57 AM Simon Burge wrote: > > Module Name:src > Committed By: simonb > Date: Mon Mar 29 01:57:09 UTC 2021 > > Modified Files: > src/distrib/sets/lists/comp: stl.mi > src/external/cddl/osnet/lib: Makefile > > Log Message: > Don't build or install /usr/lib/dtrace/drti.o - currently not used, may > be one day... > > > To generate a diff of this commit: > cvs rdiff -u -r1.7 -r1.8 src/distrib/sets/lists/comp/stl.mi > cvs rdiff -u -r1.9 -r1.10 src/external/cddl/osnet/lib/Makefile > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. >
re: CVS commit: src/external/mpl/bind/dist
> I think this is a misunderstanding. indeed. sorry for the noise and mis-request. .mrg.
Re: CVS commit: src/external/mpl/bind/dist
06.04.2021 20:55:54 matthew green : >> Module Name: src >> Committed By: rillig >> Date: Mon Apr 5 11:27:04 UTC 2021 >> >> Modified Files: >> src/external/mpl/bind/dist/bin/check: check-tool.c named-checkconf.c >> named-checkzone.c > [ ... ] >> src/external/mpl/bind/dist/lib/ns/tests: nstest.h >> >> Log Message: >> bind: remove unnecessary CONSTCOND comments >> >> Since lint1/tree.c 1.202 from 2021-01-31, lint no longer needs the >> /*CONSTCOND*/ for do-while-0 "loops". >> >> No functional change. > > please do not make such changes to upstream code. it should > be obvious why.. > > please revert this and all following changes. I think this is a misunderstanding. My recent commits made our local copy as similar to the upstream code as possible. Before my changes, our copy differed because we had inserted the /*CONSTCOND*/ comments, plus a few workarounds for bugs in lint. Since these bugs have been fixed a few days ago, we no longer need these comments and workarounds, therefore I removed them. When I wrote the commit message, I didn't mention these details since in that very moment they were obvious to me. In that moment I didn't think about this possible misunderstanding. Roland
Re: CVS commit: src/external/mpl/bind/dist
In article <9374.1617735...@splode.eterna.com.au>, matthew green wrote: >> Module Name: src >> Committed By:rillig >> Date:Mon Apr 5 11:27:04 UTC 2021 >> >> Modified Files: >> src/external/mpl/bind/dist/bin/check: check-tool.c named-checkconf.c >> named-checkzone.c >[ ... ] >> src/external/mpl/bind/dist/lib/ns/tests: nstest.h >> >> Log Message: >> bind: remove unnecessary CONSTCOND comments >> >> Since lint1/tree.c 1.202 from 2021-01-31, lint no longer needs the >> /*CONSTCOND*/ for do-while-0 "loops". >> >> No functional change. > >please do not make such changes to upstream code. it should >be obvious why.. > >please revert this and all following changes. This is reversion of our local changes to match upstream. So those patches actually do what you are asking :-) christos
re: CVS commit: src/external/mpl/bind/dist
> Module Name: src > Committed By: rillig > Date: Mon Apr 5 11:27:04 UTC 2021 > > Modified Files: > src/external/mpl/bind/dist/bin/check: check-tool.c named-checkconf.c > named-checkzone.c [ ... ] > src/external/mpl/bind/dist/lib/ns/tests: nstest.h > > Log Message: > bind: remove unnecessary CONSTCOND comments > > Since lint1/tree.c 1.202 from 2021-01-31, lint no longer needs the > /*CONSTCOND*/ for do-while-0 "loops". > > No functional change. please do not make such changes to upstream code. it should be obvious why.. please revert this and all following changes. .mrg.
Re: CVS commit: src/tests/lib/libcurses/director
On Tue, Apr 06, 2021 at 00:47:00 +, Roland Illig wrote: > The previous "table" was an insult to any reader. It was unsorted, > listed the functions shuffled, and was not even formatted consistently. That's pretty rude. Please, tone down your commit "messages". -uwe
Re: CVS commit: src/sys/conf
"Christos Zoulas" wrote: > Module Name: src > Committed By: christos > Date: Mon Apr 5 22:52:03 UTC 2021 > > Modified Files: > > src/sys/conf: Makefile.kern.inc > > Log Message: > > Don't use /usr/bin/time (it is not portable) Oops, that bit wasn't meant to sneak in. Thanks for noticing and fixing. Cheers, Simon.
Re: CVS commit: src/usr.bin/at
Robert Elz wrote: > Date:Fri, 2 Apr 2021 06:31:53 + > From:"Simon Burge" > Message-ID: <20210402063153.773c7f...@cvs.netbsd.org> > > | Add an XXX reminder to convert at(1) to use parsedate(3) in . > > If that's intended as an optional facility (at -d ... or something), > then fine, but please don't make parsedate the standard way to input > at dates (just as it isn't the standard way to set the date using date(1), > it is way too fragile and cantankerous for something that should be reliable. That's good to know about parsedate(3). I hadn't considered that what looked more flexible could also be fragile and cantankerous :). I'll revert that comment. Cheers, Simon.
Re: CVS commit: src/usr.bin/at
Date:Fri, 2 Apr 2021 06:31:53 + From:"Simon Burge" Message-ID: <20210402063153.773c7f...@cvs.netbsd.org> | Add an XXX reminder to convert at(1) to use parsedate(3) in . If that's intended as an optional facility (at -d ... or something), then fine, but please don't make parsedate the standard way to input at dates (just as it isn't the standard way to set the date using date(1), it is way too fragile and cantankerous for something that should be reliable. kre
Re: CVS commit: src/sys/arch
"Rin Okuyama" wrote: > Module Name: src > Committed By: rin > Date: Fri Apr 2 12:11:42 UTC 2021 > > Log Message: > > For ports with __HAVE_LEGACY_INTRCNT, turn intrcnt[] and derived > variables into u_int, to match with kern/subr_evcnt.c. Thanks Rin! Cheers, Simon.
Re: CVS commit: src/sys/arch/powerpc/oea
Ugh. Can we please stop making these hacky one-offs in "shared by all PowerPC platforms" code? This actually points to a deeper problem in the pmap code that needs to be addressed correctly. > On Apr 1, 2021, at 3:02 PM, Michael Lorenz wrote: > > Module Name: src > Committed By: macallan > Date: Thu Apr 1 22:02:20 UTC 2021 > > Modified Files: > src/sys/arch/powerpc/oea: ofwoea_machdep.c > > Log Message: > avoid mapping 0xf000 - my beige G3 DSIs on it > with this my the machine boots again > tested on a variety of G4 and G5 models with no problems > > > To generate a diff of this commit: > cvs rdiff -u -r1.59 -r1.60 src/sys/arch/powerpc/oea/ofwoea_machdep.c > > Please note that diffs are not public domain; they are subject to the > copyright notices on the relevant files. > -- thorpej
Re: CVS commit: src/share/misc
On Fri, Apr 02, 2021 at 12:45:28AM +0200, Roland Illig wrote: > For simple cases I agree with Matthew that parentheses should not be > required. Looking through the src tree, I noticed though that the vast > majority of the code uses the redundant parentheses, so I also agree > with Christos that it's good to have a simple rule. I would strongly prefer if no new code gets added using the old style, but I also don't want to see anyone going around to just do whitespace^W parenthesis cleanups. Joerg
Re: CVS commit: src/share/misc
On 31.03.2021 11:03, Robert Elz wrote: Here, as I recall it, the issue only arose because of something either lint or indent was doing, or wanted to do. I don't recall which, but I do not remember any earlier complaints from anyone that they couldn't understand code because of missing () around a variable name arg to sizeof. It was neither lint nor indent that did anything, it was just me editing the lint code. I had removed the parentheses where not required by the syntax since I felt them to be misleading since these parentheses are not part of a function call expression but are instead just the usual parentheses for grouping expressions. I had read share/misc/style before, but interpreted it differently, thinking that it only preferred 'sizeof(x)' over 'sizeof (x)' and that this guideline would not say anything about whether there should be parentheses or not. For simple cases I agree with Matthew that parentheses should not be required. Looking through the src tree, I noticed though that the vast majority of the code uses the redundant parentheses, so I also agree with Christos that it's good to have a simple rule. Roland
Re: CVS commit: src/share/misc
Date:Wed, 31 Mar 2021 03:03:53 - (UTC)
From:chris...@astron.com (Christos Zoulas)
Message-ID:
| There are 3 x 'sizeof(' in the tree compared to 'sizeof ' in '*.c' and
| I am counting 'sizeof (' as 'sizeof ':
|
| 191337 'sizeof('
| 63508 'sizeof '
|
| I think that it is better to bless the prevailing majority as the rule,
| but we should let others express their opinions first.
That's not a reasonable count, as it ignores (or rather, includes) the
cases where sizeof requires the '(' - all that should be being counted,
that is, if this were a reasonable way to decide anything, would be the
uses of sizeof where the parentheses are optional.
Personally, my main preference would be for fewer rules. If there isn't
a really good reason for there to be a rule, there shouldn't be one.
Here, as I recall it, the issue only arose because of something either
lint or indent was doing, or wanted to do. I don't recall which, but
I do not remember any earlier complaints from anyone that they couldn't
understand code because of missing () around a variable name arg to sizeof.
If the instigator was lint, then it should simply ignore this issue (and
all other issues which are simply style) - if it encounters something which
is truly potentially ambiguous, sure, issue a warning (but with some easy
method to shut it up if the warning is bogus), but otherwise, nothing.
If it was indent, then I don't much care what it does, but I'd prefer it to
leave most hand crafted code alone, unless it really has to be fixed. I
see no compelling reason for it to ever fiddle the arg to sizeof (except
perhaps if someone inserted a line break between the two).
For the issue in question, my personal preference is for sizeof variable to
not have parentheses (including in cases like sizeof *ptr, sizeof ptr->field
and sizeof struct.field), unless they really are needed for readability, or
accuracy. Parentheses are required for sizeof(type) of course, I don't
much care whether there's a space between 'sizeof' and '(' or not, I'd leave
that to whoever is inserting the code - the only caveat being to retain the
style of the original (if other uses have a space, use it, if they don't,
omit it). I prefer not to do sizeof (expression) at all, but in the very
rare case it is needed, the parentheses are required (of course), and in those
unless there's a good reason not to (line length is one) I'd normally include
the space before the '('.
But those are just my preferences, and when I modify code, I follow the style
of the original (I even used Steve Bourne's algol68-like crap when doing some
minor modifications to adb long long long ago).
I see no need for a rule. Just leave things as they were a month ago, and
we can all remain happy.
kre
Re: CVS commit: src/share/misc
On Wed, Mar 31, 2021 at 12:38:20PM +1100, matthew green wrote: > i already did in the other thread -- apply the existing > () rule. aka, avoid it unless it helps comprehension, > which means simple sizeof can avoid it, but anything > slightly complex should not. this means that all the > fun cases will use () and the specific case i won't use > it for is left alone (snprintf(buf, sizeof buf, ...)). I am in favor of this, for exactly that usage (i.e. passing a buffer and its size to a function that takes a pointer / size_t argument). However, all cases where the paranthesis pretend a different evaluation order should be avoided anyway. Martin
Re: CVS commit: src/share/misc
In article <6734.1617154...@splode.eterna.com.au>,
matthew green wrote:
>Christos Zoulas writes:
>> In article <20900.1616977...@splode.eterna.com.au>,
>> matthew green wrote:
>> >> Log Message:
>> >> Clarify and explain the rationale for parentheses in sizeof and return as
>> >> discussed.
>> >
>> >+* a function call. We always parenthesize the sizeof expression for
>> >+* consistency.
>> >
>> >i object. this discussion was not finished.
>>
>> Ok, please provide an alternative proposal.
>
>i already did in the other thread -- apply the existing
>() rule. aka, avoid it unless it helps comprehension,
>which means simple sizeof can avoid it, but anything
>slightly complex should not. this means that all the
>fun cases will use () and the specific case i won't use
>it for is left alone (snprintf(buf, sizeof buf, ...)).
There are 3 x 'sizeof(' in the tree compared to 'sizeof ' in '*.c' and
I am counting 'sizeof (' as 'sizeof ':
191337 'sizeof('
63508 'sizeof '
I think that it is better to bless the prevailing majority as the rule,
but we should let others express their opinions first.
Best,
christos
re: CVS commit: src/share/misc
Christos Zoulas writes: > In article <20900.1616977...@splode.eterna.com.au>, > matthew green wrote: > >> Log Message: > >> Clarify and explain the rationale for parentheses in sizeof and return as > >> discussed. > > > >+* a function call. We always parenthesize the sizeof expression for > >+* consistency. > > > >i object. this discussion was not finished. > > Ok, please provide an alternative proposal. i already did in the other thread -- apply the existing () rule. aka, avoid it unless it helps comprehension, which means simple sizeof can avoid it, but anything slightly complex should not. this means that all the fun cases will use () and the specific case i won't use it for is left alone (snprintf(buf, sizeof buf, ...)). thanks. .mrg.
