Re: FYI: ^T use during poudriere bulk vs. /bin/sh operation: I got a "Unsafe ckmalloc() call" Abort trap that left a mess
On Mon, Sep 04, 2023 at 05:16:56PM -0700, Mark Millard wrote: > During a (zfs based) poudriere bulk -a run a ^T got a: > Unsafe ckmalloc() call > Abort trap (core dumped) > My attribution to ^T handling is unverified: I did not find the > sh.core file. It is just what the timing looked like. The error message means that ckmalloc() is being called without INTOFF in effect, i.e. at the time a SIGINT may cause an EXINT exception (longjmp()). Although malloc(3)'s data structures could be protected by surrounding the malloc() call with INTOFF and INTON, this would lead to a memory leak if a SIGINT happened at that time, since the pointer to the allocated memory would be lost. This check was added in git commit 9f9c9549fd4f7ce362e95e3a8a50f00ffd00175c. My first guess would be that there is a bug with a rare edge case of traps and/or errors, such as not applying INTOFF again after an exception has turned it off or doing INTON when interrupts are already enabled. A less likely possibility could be a violation related to volatile and synchronization between a signal handler and the main flow. Many common code paths are all exercised by the tests and normal use, so it must be something special in some way. -- Jilles Tjoelker
Re: should a copy_file_range(2) syscall be interrupted via a signal
On Fri, Jul 05, 2019 at 12:28:51AM +, Rick Macklem wrote:
> I have been working on a Linux compatible copy_file_range(2) syscall
> (the current code can be found at https://reviews.freebsd.org/D20584).
> One outstanding issue is how it should deal with signals. Right now, I
> have vn_start_write() without PCATCH, so that it won't be interrupted
> by a signal, but I notice that vn_write() {ie. write syscall } does
> have PCATCH on vn_start_write() and so does vn_rdwr() when it is
> called without IO_NODELOCKED.
A regular write() is only interruptible when writing to a terminal,
pseudo-terminal master, pipe, socket, or, under certain conditions, a
file on an NFS intr mount. Therefore, applications may not have the code
to resume interrupted writes to regular files gracefully.
> I am thinking that copy_file_range(2) should do this also.
> However, if it returns an error, it is impossible for the caller to
> know how much of the data range got copied.
A regular write() returns partial success if interrupted by a signal
when it has already written something. Therefore, the application can
resume the operation by adjusting pointers and counts.
Something similar applies to "deterministic" errors like [EFBIG] where
the first call will write as far as possible (if this is not nothing)
successfully and the next attempt will return the error.
> What do you think the copy_file_range(2) code should do?
I'm not sure it should actually be done, but the need for adjusting
pointers and counts could be avoided with a little extra kernel and libc
code. The system call would receive an additional argument pointing to
an off_t that indicates how many bytes previous calls have already
written. A libc wrapper would initialize this to 0. With this, the
system call can be restarted automatically after a signal.
In any case, [EINTR] and the internal ERESTART must not be returned
unless it is safe to repeat the call with the same (direct) arguments.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: A reliable port cross-build failure (hangup) in my context (amd64->armv7 cross build, with native-tool speedup involved)
es() and struct dirent.d_off with filesystems that return hashed filenames as positions. > - if host syscall returns ERESTART, we should do full unroll and pass it > to guest. Yes (with the above mentioned caveats about how ERESTART is returned). > - the syscalls emulation should not use the libc functions, but syscall > instruction directly. Libc shims can have side effects so we should not > to execute it twice. Once in guest, second time in host. If you accept that your code is going to be more tightly coupled to libc and the kernel than most applications, calling system calls directly should be fine. This will also allow you to install your own handler for SIGTHR if you do not want to remap it. Do not expect pthread cancellation and suspension to work properly in such a configuration, though. > - and last major one. At this time, all guest structures are maintained > by hand. Due to huge amount of these structures, this is the extreme > error prone approach. We should convert this to script generated code, > including guest syscalls definition. Definitions of system calls are in syscalls.master and should be automatically processable; definitions of types are in header files and cannot really be processed other than by a C compiler. > Again, my apology for slightly (or much) chaotic report, but this is the > best what's I capable. It was clear enough to me. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: FYI: devel/kyua 14 failures for head -r338518M based build in a Pine64+ 2GB (aarch64 / cortexA53 / A64) context
On Sun, Sep 16, 2018 at 01:21:33PM -0700, Mark Millard wrote: > On 2018-Sep-16, at 10:50 AM, Jilles Tjoelker wrote: > > On another note, the comment just below that, > >/* But we need to zero-extend (char is unsigned) the value and then > > perform a signed 32-bit subtraction. */ > > shows a wrong reason for doing the right thing since memcmp (as well as > > strcmp and strncmp) are defined to compare based on unsigned chars, > > regardless of the signedness of char. > Ahh, standard are so much fun: there are so many to choose from. > I looked up ISO/IEC 9899:2011 (E) and its 7.24.4.1 "The memcmp function". > It makes no such explicit claim about using using unsigned chars for > the comparison standard: > QUOTE of the Description part: > The memcmp function compares the first n characters of the object pointed > by s1 to the first n characters of the object pointed by s2. > END QUOTE > QUOTE of the Returns part: > The memcmp function returns an integer greater than, equal to, or less > than zero, accordingly as the object pointed to by s1 is greater than, > equal to, or less than the object pointed to by s2. > END QUOTE > If I had to guess the intent of "characters" would be based on the > char type for C11. I did not find anything about the issue in the C99 > rationale that I also happen to have available to look at. In C99, C11 and C17, the text about using unsigned chars is under "Comparison functions" and it applies to memcmp, strcmp and strncmp (which are documented together in the section). > For all I know, POSIX or other standards may be more explicit and not > agree. POSIX does not document memcmp, strcmp and strncmp close together and copies the text about using unsigned chars to each of them. This means that it agrees with the C standards. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: FYI: devel/kyua 14 failures for head -r338518M based build in a Pine64+ 2GB (aarch64 / cortexA53 / A64) context
On Tue, Sep 11, 2018 at 08:48:03AM -0700, Mark Millard wrote: > [Adding listing broken tests, but ignoring sys/cddl/zfs/ ones. > lib/libc/string/memcmp_test:diff is one of them.] > ===> Broken tests > lib/libc/string/memcmp_test:diff -> broken: Premature exit; test case > received signal 6 (core dumped) [3.962s] The problem here is that our definition of memcmp() is tighter than the one in the standards and glibc. We define the return value to be the difference between the first differing bytes, while the standards and glibc only define the sign (negative, zero or positive). Looking at contrib/cortex-strings/src/aarch64/memcmp.S, a bic pos, pos, #7 after the clz may help. On another note, the comment just below that, /* But we need to zero-extend (char is unsigned) the value and then perform a signed 32-bit subtraction. */ shows a wrong reason for doing the right thing since memcmp (as well as strcmp and strncmp) are defined to compare based on unsigned chars, regardless of the signedness of char. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Getting PID of socket client
On Sun, Jul 09, 2017 at 02:07:06PM +, Johannes Lundberg wrote: > That code gets the child's pid from the fork command. That's is not > available in my case (Wayland client connects to Wayland server via unix > socket). > I can understand the security issue.. hmm will dig further tomorrow.. It seems like what Wayland wants is not a security feature, but a feature to prevent people from building things that will not work in a future more secure world. In this future world, operations like making a screenshot would be privileged. Even if the PID race is solved, it remains trivial to fake the check (for example, fork a process that sends the initial message and then immediately execs a "privileged" binary, or use ptrace to attach to a "privileged" binary or launch a new copy of a "privileged" binary). With regard to security, it would be equivalent to have the client send the name of its binary to the server. Putting this into a low-level Wayland library would deter people from faking the check to do things that will not work in the future more secure world. I don't know how invasive this would be, though. One possible implementation of the future more secure world would be per-application UIDs a la Android. Another one would be Capsicum-sandboxed applications where applications receive their Wayland sockets pre-connected by code that tells the Wayland server the application identity. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Getting PID of socket client
On Sun, Jul 09, 2017 at 05:43:22PM +0100, David Chisnall wrote: > On 9 Jul 2017, at 14:25, Stefan Ehmann <shoes...@gmx.net> wrote: > > Don't why the structs are not compatible, maybe because: > > "The process ID cmcred_pid should not be looked up (such as via the > > KERN_PROC_PID sysctl) for making security decisions. The sending > > process could have exited and its process ID already been reused for > > a new process." > Note that having the kernel provide a process descriptor instead of a > PID would allow the userspace process to have race-free access to the > PID. This is an interesting idea, but would require quite a few changes. First, current process descriptors act as an artificial parent process, suppressing the normal SIGCHLD to the parent and not being matched by a wildcard waitpid() or similar function. A new kind of process descriptor would have to be added which leaves this behaviour unchanged and could exist in parallel with a process descriptor from pdfork(). Second, pdgetpid() makes no guarantees whether the process ID still exists. It should not make them either for this case, since this would allow another user to hold onto process slots for RLIMIT_NPROC. The only solution would be to add variants of the necessary calls that take a process descriptor instead of a process ID. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 05:25:35PM +0300, Konstantin Belousov wrote: > On Sun, May 21, 2017 at 04:03:55PM +0200, Jilles Tjoelker wrote: > > On Sun, May 21, 2017 at 03:31:18PM +0300, Konstantin Belousov wrote: > > > On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote: > > > > We have another type in this area which is too small in some situations: > > > > uint8_t for struct dirent.d_namlen. For filesystems that store filenames > > > > as upto 255 UTF-16 code units, the name to be stored in d_name may be > > > > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code > > > > currently handles this by returning the short (8.3) name, but this name > > > > may not be present or usable, leaving the file inaccessible. > > > > Actually allowing longer names seems too complicated to add to the ino64 > > > > change, but changing d_namlen to uint16_t (using d_pad0 space) and > > > > skipping entries with d_namlen > 255 in libc may be helpful. > > > > Note that applications using the deprecated readdir_r() will not be able > > > > to read such long names, since the API does not allow specifying that a > > > > larger buffer has been provided. (This could be avoided by making struct > > > > dirent.d_name 766 bytes long instead of 256.) > > > > Unfortunately, the existence of readdir_r() also prevents changing > > > > struct dirent.d_name to the more correct flexible array. > > > Yes, changing the size of d_name at this stage of the project is out of > > > question. My reading of your proposal is that we should extend the size > > > of d_namlen to uint16_t, am I right ? Should we go to 32bit directly > > > then, perhaps ? > > Yes, my proposal is to change d_namlen to uint16_t. > > Making it 32 bits is not useful with the 16-bit d_reclen, and increasing > > d_reclen does not seem useful to me with the current model of > > getdirentries() where the whole dirent must fit into the caller's > > buffer. > Bumping it now might cause less churn later, even if unused, but ok. > > > I did not committed the change below, nor did I tested or even build it. > > I'd like to skip overlong names in the native readdir_r() as well, so > > that long name support can be added to the kernel later without causing > > buffer overflows with applications using FreeBSD 12.0 libc. > > The native readdir() does not seem to have such a problem. > Again, not even compiled. Looks good to me. > [patch snipped] -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Sun, May 21, 2017 at 03:31:18PM +0300, Konstantin Belousov wrote: > On Sun, May 21, 2017 at 02:14:56PM +0200, Jilles Tjoelker wrote: > > We have another type in this area which is too small in some situations: > > uint8_t for struct dirent.d_namlen. For filesystems that store filenames > > as upto 255 UTF-16 code units, the name to be stored in d_name may be > > upto 765 bytes long in UTF-8. This was reported in PR 204643. The code > > currently handles this by returning the short (8.3) name, but this name > > may not be present or usable, leaving the file inaccessible. > > Actually allowing longer names seems too complicated to add to the ino64 > > change, but changing d_namlen to uint16_t (using d_pad0 space) and > > skipping entries with d_namlen > 255 in libc may be helpful. > > Note that applications using the deprecated readdir_r() will not be able > > to read such long names, since the API does not allow specifying that a > > larger buffer has been provided. (This could be avoided by making struct > > dirent.d_name 766 bytes long instead of 256.) > > Unfortunately, the existence of readdir_r() also prevents changing > > struct dirent.d_name to the more correct flexible array. > Yes, changing the size of d_name at this stage of the project is out of > question. My reading of your proposal is that we should extend the size > of d_namlen to uint16_t, am I right ? Should we go to 32bit directly > then, perhaps ? Yes, my proposal is to change d_namlen to uint16_t. Making it 32 bits is not useful with the 16-bit d_reclen, and increasing d_reclen does not seem useful to me with the current model of getdirentries() where the whole dirent must fit into the caller's buffer. > I did not committed the change below, nor did I tested or even build it. I'd like to skip overlong names in the native readdir_r() as well, so that long name support can be added to the kernel later without causing buffer overflows with applications using FreeBSD 12.0 libc. The native readdir() does not seem to have such a problem. > [patch snipped] -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: 64-bit inodes (ino64) Status Update and Call for Testing
On Thu, Apr 20, 2017 at 10:43:14PM +0300, Konstantin Belousov wrote: > Inodes are data structures corresponding to objects in a file system, > such as files and directories. FreeBSD has historically used 32-bit > values to identify inodes, which limits file systems to somewhat under > 2^32 objects. Many modern file systems internally use 64-bit identifiers > and FreeBSD needs to follow suit to properly and fully support these > file systems. > The 64-bit inode project, also known as ino64, started life many years > ago as a project by Gleb Kurtsou (gleb@). After that time several > people have had a hand in updating it and addressing regressions, after > mckusick@ picked up and updated the patch, and acted as a flag-waver. > Sponsored by the FreeBSD Foundation I have spent a significant effort > on outstanding issues and integration -- fixing compat32 ABI, NFS and > ZFS, addressing ABI compat issues and investigating and fixing ports > failures. rmacklem@ provided feedback on NFS changes, emaste@ and > jhb@ provided feedback and review on the ABI transition support. pho@ > performed extensive testing and identified a number of issues that > have now been fixed. kris@ performed an initial ports investigation > followed by an exp-run by antoine@. emaste@ helped with organization > of the process. > This note explains how to perform useful testing of the ino64 branch, > beyond typical smoke tests. > 1. Overview. > The ino64 branch extends the basic system types ino_t and dev_t from > 32-bit to 64-bit, and nlink_t from 16-bit to 64-bit. The struct dirent > layout is modified due to the larger size of ino_t, and also gains a > d_off (directory offset) member. As ino64 implies an ABI change anyway > the struct statfs f_mntfromname[] and f_mntonname[] array length > MNAMELEN is increased from 88 to 1024, to allow for longer mount path > names. > ABI breakage is mitigated by providing compatibility using versioned > symbols, ingenious use of the existing padding in structures, and by > employing other tricks. Unfortunately, not everything can be fixed, > especially outside the base system. For instance, third-party APIs > which pass struct stat around are broken in backward and forward- > incompatible way. We have another type in this area which is too small in some situations: uint8_t for struct dirent.d_namlen. For filesystems that store filenames as upto 255 UTF-16 code units, the name to be stored in d_name may be upto 765 bytes long in UTF-8. This was reported in PR 204643. The code currently handles this by returning the short (8.3) name, but this name may not be present or usable, leaving the file inaccessible. Actually allowing longer names seems too complicated to add to the ino64 change, but changing d_namlen to uint16_t (using d_pad0 space) and skipping entries with d_namlen > 255 in libc may be helpful. Note that applications using the deprecated readdir_r() will not be able to read such long names, since the API does not allow specifying that a larger buffer has been provided. (This could be avoided by making struct dirent.d_name 766 bytes long instead of 256.) Unfortunately, the existence of readdir_r() also prevents changing struct dirent.d_name to the more correct flexible array. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: r314708: panic: tdsendsignal: ksi on queue
On Thu, Mar 09, 2017 at 04:46:46PM +0200, Konstantin Belousov wrote:
> Yes, there is a race, apparently, with the child zombie still not finishing
> sending the SIGCHLD to the parent and parent exiting. The following should
> fix the issue, but I do not think that reproducing the problem is easy.
> diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
> index c524fe5df37..ba5ff84e9de 100644
> --- a/sys/kern/kern_exit.c
> +++ b/sys/kern/kern_exit.c
> @@ -189,6 +189,7 @@ exit1(struct thread *td, int rval, int signo)
> {
> struct proc *p, *nq, *q, *t;
> struct thread *tdt;
> + ksiginfo_t ksi;
>
> mtx_assert(, MA_NOTOWNED);
> KASSERT(rval == 0 || signo == 0, ("exit1 rv %d sig %d", rval, signo));
> @@ -456,7 +457,12 @@ exit1(struct thread *td, int rval, int signo)
> proc_reparent(q, q->p_reaper);
> if (q->p_state == PRS_ZOMBIE) {
> PROC_LOCK(q->p_reaper);
> - pksignal(q->p_reaper, SIGCHLD, q->p_ksi);
> + if (q->p_ksi != NULL) {
> + ksiginfo_init();
> + ksiginfo_copy(q->p_ksi, );
> + }
> + pksignal(q->p_reaper, SIGCHLD, q->p_ksi !=
> + NULL ? : NULL);
> PROC_UNLOCK(q->p_reaper);
> }
> } else {
This patch introduces a subtle correctness bug. A real SIGCHLD ksiginfo
should always be the zombie's p_ksi; otherwise, the siginfo may be lost
if there are too many signals pending for the target process or in the
system. If the siginfo is lost and the reaper normally passes si_pid to
waitpid() or similar (instead of passing WAIT_ANY or P_ALL), a zombie
will remain until the reaper terminates.
Conceptually the siginfo is sent to one process at a time only, so the
bug is an artifact of the implementation. Perhaps the piece of code
added in r309886 can be moved or the ksiginfo can be removed from the
parent's queue.
If such a fix is not possible, it may be better to send a bare SIGCHLD
(si_code is SI_KERNEL or 0, depending on how many signals are pending)
in this situation and document that reapers must use WAIT_ANY or P_ALL.
(However, compared to the pre-r309886 situation they can still use
SIGCHLD to get notified when to call waitpid() or similar.)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Fix /etc/rc.d/random umask handling (/entropy permissions)
On Mon, Jan 23, 2017 at 10:52:21AM -0800, Simon J. Gerraty wrote:
> Jilles Tjoelker <jil...@stack.nl> wrote:
> > Index: etc/rc.d/random
> > ===
> > --- etc/rc.d/random (revision 311446)
> > +++ etc/rc.d/random (working copy)
> > @@ -20,12 +20,14 @@
> >
> > save_dev_random()
> > {
> > + oumask=`umask`
> why not simply use a sub-shell to tighten umask
> (umask 077; what-ever)
With our /bin/sh, the save-restore method saves a fork. A command
substitution with a single umask command does not fork, while a subshell
containing umask and something else does.
The effect is fairly minor, but good performance is often the product of
many small optimizations.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Fix /etc/rc.d/random umask handling (/entropy permissions)
On Sun, Jan 22, 2017 at 01:22:07AM +, Lu Tung-Pin wrote: > On 2017-01-21 22:01, Jilles Tjoelker wrote: > > [Adding Cc: Dag-Erling Smørgrav who committed r273957 which seems to > > have introduced this] > > On Sat, Jan 21, 2017 at 01:21:42AM +, Lu Tung-Pin wrote: > >> A 2014 change broke the umask handling in /etc/rc.d/random, > >> leaving /entropy with ug+r permissions. Quick fix attached, > Edit: go+r permissions. > > Switching the umask here will avoid incorrect permissions on > > /entropy on new installations, but will not fix existing systems. A > > chmod command may be useful here. > Note that random_start() first removes /entropy via feed_dev_random(). > There's also a removal in random_stop(). Provided that a removal occurs, > the chmod won't be necessary on machines with an existing go+r /entropy. Right, /entropy is deleted after being read so the chmod is not needed. > I'm wondering, though: Would it be better to replace all the umask > fiddling with simple chmods? Every other rc.d script uses chmod if it > needs to set tighter permissions. When umask is used (dmesg, mountd, > syslogd), it's with a relaxed 022 setting. The umask ensures the file is created with the correct permissions so there is no race window where an unprivileged process can open the file. A permissions change has no existing opens. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Fix /etc/rc.d/random umask handling (/entropy permissions)
[Adding Cc: Dag-Erling Smørgrav who committed r273957 which seems to
have introduced this]
On Sat, Jan 21, 2017 at 01:21:42AM +, Lu Tung-Pin wrote:
> A 2014 change broke the umask handling in /etc/rc.d/random,
> leaving /entropy with ug+r permissions. Quick fix attached,
> mirroring random_stop() behavior.
> (Incidentally, /usr/libexec/save-entropy is still fine for
> /var/db/entropy/*, as is /etc/rc.d/random for the new
> /boot/entropy.)
> --- /etc/rc.d/random.old 2017-01-21 11:48:30.975009000 +1100
> +++ /etc/rc.d/random 2017-01-19 18:04:34.224632000 +1100
> @@ -20,12 +20,15 @@
>
> save_dev_random()
> {
> + oumask=`umask`
> + umask 077
> for f ; do
> if :>>"$f" ; then
> debug "saving entropy to $f"
> dd if=/dev/random of="$f" bs=4096 count=1 2>/dev/null
> fi
> done
> + umask ${oumask}
> }
>
> feed_dev_random()
Switching the umask here will avoid incorrect permissions on /entropy on
new installations, but will not fix existing systems. A chmod command
may be useful here.
On another note, if :>>"$f" is bogus. Since : is a special builtin, a
redirection error causes the shell to abort the script. The conditional
seems to have been added to show error messages when the entropy file
cannot be written without showing dd's statistics. I think this can be
done more easily using dd's status=none parameter.
My revised patch is below:
Index: etc/rc.d/random
===
--- etc/rc.d/random (revision 311446)
+++ etc/rc.d/random (working copy)
@@ -20,12 +20,14 @@
save_dev_random()
{
+ oumask=`umask`
+ umask 077
for f ; do
- if :>>"$f" ; then
- debug "saving entropy to $f"
- dd if=/dev/random of="$f" bs=4096 count=1 2>/dev/null
- fi
+ debug "saving entropy to $f"
+ dd if=/dev/random of="$f" bs=4096 count=1 status=none &&
+ chmod 600 "$f"
done
+ umask ${oumask}
}
feed_dev_random()
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: copyinstr and ENAMETOOLONG
On Fri, Dec 02, 2016 at 10:20:32AM -0600, Eric van Gyzen wrote: > On 11/02/2016 15:33, Jilles Tjoelker wrote: > > On Wed, Nov 02, 2016 at 02:24:43PM -0500, Eric van Gyzen wrote: > >> Does copyinstr guarantee that it has filled the output buffer when it > >> returns ENAMETOOLONG? I usually try to answer my own questions, but I > >> don't speak many dialects of assembly. :) > > > > The few I checked appear to work by checking the length while copying, > > but the man page copy(9) does not guarantee that, and similar code in > > sys/compat/linux/linux_misc.c linux_prctl() LINUX_PR_SET_NAME performs a > > copyin() if copyinstr() fails with [ENAMETOOLONG]. > Thanks. > > In implementing copyinstr(), checking the length first may make sense > > for economy of code: a user-strnlen() using an algorithm like > > lib/libc/string/strlen.c and a copyin() connected together with a C > > copyinstr(). This would probably be faster than the current amd64 code > > (which uses lods and stos, meh). With that implementation, filling the > > buffer in the [ENAMETOOLONG] case requires a small piece of additional > > code. > >> I ask because I'd like to make the following change, and I'd like to > >> know whether I should zero the buffer before calling copyinstr to ensure > >> that I don't set the thread's name to the garbage that was on the stack. > [snip] > > For API design, it makes more sense to set error = 0 if a truncated name > > is being set anyway. This preserves the property that the name remains > > unchanged if the call fails. > That makes sense. > > A change to the man page thr_set_name(2) is needed in any case. > Of course. > Would you like to review the following? > Index: lib/libc/sys/thr_set_name.2 > === > --- lib/libc/sys/thr_set_name.2 (revision 309300) > +++ lib/libc/sys/thr_set_name.2 (working copy) > @@ -28,7 +28,7 @@ > .\" > .\" $FreeBSD$ > .\" > -.Dd June 1, 2016 > +.Dd December 2, 2016 > .Dt THR_SET_NAME 2 > .Os > .Sh NAME > @@ -43,37 +43,34 @@ > .Sh DESCRIPTION > The > .Fn thr_set_name > -sets the user-visible name for the kernel thread with the identifier > +system call sets the user-visible name for the thread with the identifier > .Va id > -in the current process, to the NUL-terminated string > +in the current process to the NUL-terminated string > .Va name . > +The name will be silently truncated to fit into a buffer of > +.Dv MAXCOMLEN + 1 > +bytes. > The thread name can be seen in the output of the > .Xr ps 1 > and > .Xr top 1 > commands, in the kernel debuggers and kernel tracing facility outputs, > -also in userland debuggers and program core files, as notes. > +and in userland debuggers and program core files, as notes. > .Sh RETURN VALUES > If successful, > .Fn thr_set_name > -will return zero, otherwise \-1 is returned, and > +returns zero; otherwise, \-1 is returned, and > .Va errno > is set to indicate the error. > .Sh ERRORS > The > .Fn thr_set_name > -operation may return the following errors: > +system call may return the following errors: > .Bl -tag -width Er > .It Bq Er EFAULT > The memory pointed to by the > .Fa name > argument is not valid. > -.It Bq Er ENAMETOOLONG > -The string pointed to by the > -.Fa name > -argument exceeds > -.Dv MAXCOMLEN + 1 > -bytes in length. > .It Bq Er ESRCH > The thread with the identifier > .Fa id > @@ -92,6 +89,6 @@ does not exist in the current process. > .Xr ktr 9 > .Sh STANDARDS > The > -.Fn thr_new > -system call is non-standard and is used by > +.Fn thr_set_name > +system call is non-standard and is used by the > .Lb libthr . > Index: share/man/man3/pthread_set_name_np.3 > === > --- share/man/man3/pthread_set_name_np.3 (revision 309300) > +++ share/man/man3/pthread_set_name_np.3 (working copy) > @@ -24,7 +24,7 @@ > .\" > .\" $FreeBSD$ > .\" > -.Dd February 13, 2003 > +.Dd December 2, 2016 > .Dt PTHREAD_SET_NAME_NP 3 > .Os > .Sh NAME > @@ -39,14 +39,15 @@ > .Sh DESCRIPTION > The > .Fn pthread_set_name_np > -function sets internal name for thread specified by > -.Fa tid > -argument to string value specified by > +function applies a copy of the given > .Fa name > -argument. > +to the thread specified by the given > +.Fa tid . > .Sh ERRORS > Because of the debugging nature of this function, all errors that may > appear inside are silently ignored. > +.Sh SE
Re: copyinstr and ENAMETOOLONG
On Wed, Nov 02, 2016 at 02:24:43PM -0500, Eric van Gyzen wrote:
> Does copyinstr guarantee that it has filled the output buffer when it
> returns ENAMETOOLONG? I usually try to answer my own questions, but I
> don't speak many dialects of assembly. :)
The few I checked appear to work by checking the length while copying,
but the man page copy(9) does not guarantee that, and similar code in
sys/compat/linux/linux_misc.c linux_prctl() LINUX_PR_SET_NAME performs a
copyin() if copyinstr() fails with [ENAMETOOLONG].
In implementing copyinstr(), checking the length first may make sense
for economy of code: a user-strnlen() using an algorithm like
lib/libc/string/strlen.c and a copyin() connected together with a C
copyinstr(). This would probably be faster than the current amd64 code
(which uses lods and stos, meh). With that implementation, filling the
buffer in the [ENAMETOOLONG] case requires a small piece of additional
code.
> I ask because I'd like to make the following change, and I'd like to
> know whether I should zero the buffer before calling copyinstr to ensure
> that I don't set the thread's name to the garbage that was on the stack.
> Index: kern_thr.c
> ===
> --- kern_thr.c(revision 308217)
> +++ kern_thr.c(working copy)
> @@ -580,8 +580,13 @@ sys_thr_set_name(struct thread *td, struct thr_set
> if (uap->name != NULL) {
> error = copyinstr(uap->name, name, sizeof(name),
> NULL);
> - if (error)
> - return (error);
> + if (error) {
> + if (error == ENAMETOOLONG) {
> + name[sizeof(name) - 1] = '\0';
> + } else {
> + return (error);
> + }
> + }
> }
> p = td->td_proc;
> ttd = tdfind((lwpid_t)uap->id, p->p_pid);
For API design, it makes more sense to set error = 0 if a truncated name
is being set anyway. This preserves the property that the name remains
unchanged if the call fails.
A change to the man page thr_set_name(2) is needed in any case.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Weirdness in TTY & VT
On Fri, Sep 02, 2016 at 05:12:12PM -0700, Lundberg, Johannes wrote: > I'm porting some Linux code and have some weird behavior. > In this (Linux) code fstat() and minor() is used on a /dev/tty file > descriptor to get a tty number to map to. In FreeBSD this returns a number > around 60-70 for st_rdev which means VT_ACTIVATE ioctl call will fail > because it only allows values 0-12. > Should not this be compatible with FreeBSD? If not, what is the FreeBSD way > to do this? > Is there some implementation missing in vt? minor() and major() are obsolete in FreeBSD. A (userland) dev_t is an opaque identifier. The direct equivalent is devname_r() or fdevname_r() followed by parsing the resulting string. There is also a VT_GETINDEX ioctl but I don't know whether it works. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: PostgreSQL performance on FreeBSD
On Wed, Jun 22, 2016 at 07:26:52AM -0700, Maxim Sobolev wrote: > Konstantin, > Not if you do sem_unlink() immediately, AFAIK. And that's what PG does. So > the window of opportunity for the leakage is quite small, much smaller than > for SYSV primitives. Sorry for missing your status update message, I've > missed it somehow. True, but if your process architecture supports that, you can also put unnamed process-shared semaphores into a piece of shared memory (pad sem_t to a cache line if contention is expected). This is more natural API use (fully avoiding the leak) and uses less memory. It has been supported for a long time, at least since FreeBSD 9.0. Process-shared mutexes, condition variables, reader/writer locks, etc. are available in FreeBSD 11 but use more memory (a 1-page object per synchronization object), somewhat like named semaphores. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: thread suspension when dumping core
On Thu, Jun 09, 2016 at 07:34:55AM +0300, Konstantin Belousov wrote: > On Wed, Jun 08, 2016 at 11:17:44PM +0200, Jilles Tjoelker wrote: > > On Wed, Jun 08, 2016 at 04:56:35PM +0300, Konstantin Belousov wrote: > > > On Wed, Jun 08, 2016 at 06:35:08AM -0700, Mark Johnston wrote: > > > > On Wed, Jun 08, 2016 at 07:30:55AM +0300, Konstantin Belousov wrote: > > > > > On Tue, Jun 07, 2016 at 11:19:19PM +0200, Jilles Tjoelker wrote: > > > > > > I also wonder whether we may be overengineering things here. Perhaps > > > > > > the advlock sleep can simply turn off TDF_SBDRY. > > > > > Well, this was the very first patch suggested. I would be > > > > > fine with that, but again, out-of-tree code seems to be not > > > > > quite fine with that local solution. > > > > In our particular case, we could possibly use a similar approach. In > > > > general, it seems incorrect to clear TDF_SBDRY if the thread calling > > > > sx_sleep() has any locks held. It is easy to verify that all callers of > > > > lf_advlock() are safe in this respect, but this kind of auditing is > > > > generally hard. In fact, I believe the sx_sleep that led to the problem > > > > described in D2612 is the same as the one in my case. That is, the > > > > sleeping thread may or may not hold a vnode lock depending on context. > > > I do not think that in-tree code sleeps with a vnode lock held in > > > the lf_advlock(). Otherwise, system would hang in lock cascade by > > > an attempt to obtain an advisory lock. I think we can even assert > > > this with witness. > > > There is another sleep, which Jilles mentioned, in lf_purgelocks(), > > > called from vgone(). This sleep indeed occurs under the vnode lock, and > > > as such must be non-suspendable. The sleep waits until other threads > > > leave the lf_advlock() for the reclaimed vnode, and they should leave in > > > deterministic time due to issued wakeups. So this sleep is exempt from > > > the considerations, and TDF_SBDRY there is correct. > > > I am fine with either the braces around sx_sleep() in lf_advlock() to > > > clear TDF_SBDRY (sigdeferstsop()), or with the latest patch I sent, > > > which adds temporal override for TDF_SBDRY with TDF_SRESTART. My > > > understanding is that you prefer the later. If I do not mis-represent > > > your position, I understand why you do prefer that. > > The TDF_SRESTART change does fix some more problems such as umount -f > > getting stuck in lf_purgelocks(). > > However, it introduces some subtle issues that may not necessarily be a > > sufficient objection. > I did not see an objection in the notes below, but rather I read them > as an argument to return EINTR from the stop attempts from lf_advlock(). With these changes, the question is which bugs you want to fix and which bugs you want to leave unfixed or introduce. * The status quo sometimes deadlocks or almost deadlocks with thread suspension. * Restarting the locking syscall after thread suspension using ERESTART causes the suspended thread to be moved to the end of the queue and the locked area to be re-evaluated which violates POSIX but probably does not break applications. * Interrupting the locking syscall after thread suspension with EINTR breaks applications with the reasonable assumption that [EINTR] can only occur because of signals, whenever a locking attempt is suspended. This requires a change to the man page and probably some patches to ports. This could be avoided by allowing a thread to return to the user boundary (but not beyond) while staying in the queue but that is a fairly heavy API change. The thread would not be allowed to execute user code (a signal handler) since it may take indefinitely long before the thread would continue waiting for the lock, which matches exactly POSIX's specification of F_SETLKW not restarting after signal handlers. > > Firstly, adding this closes the door on fixing signal handling for > > fcntl(F_SETLKW). Per POSIX, any caught signal interrupts > > fcntl(F_SETLKW), even if SA_RESTART is set for the signal, and the Linux > > man page documents the same. Our man page has documented that SA_RESTART > > behaves normally with fcntl(F_SETLKW) since at least FreeBSD 2.0. This > > could normally be fixed via if (error == ERESTART) error = EINTR; but > > that is no longer possible if there are [ERESTART] errors that should > > still restart. > I added the translation to fcntl handler. > > Secondly, fcntl(F_SETLKW) restarting after a stop may actually be > > observable, contrary to what I wro
Re: thread suspension when dumping core
On Wed, Jun 08, 2016 at 04:56:35PM +0300, Konstantin Belousov wrote: > On Wed, Jun 08, 2016 at 06:35:08AM -0700, Mark Johnston wrote: > > On Wed, Jun 08, 2016 at 07:30:55AM +0300, Konstantin Belousov wrote: > > > On Tue, Jun 07, 2016 at 11:19:19PM +0200, Jilles Tjoelker wrote: > > > > I also wonder whether we may be overengineering things here. Perhaps > > > > the advlock sleep can simply turn off TDF_SBDRY. > > > Well, this was the very first patch suggested. I would be fine with that, > > > but again, out-of-tree code seems to be not quite fine with that local > > > solution. > > In our particular case, we could possibly use a similar approach. In > > general, it seems incorrect to clear TDF_SBDRY if the thread calling > > sx_sleep() has any locks held. It is easy to verify that all callers of > > lf_advlock() are safe in this respect, but this kind of auditing is > > generally hard. In fact, I believe the sx_sleep that led to the problem > > described in D2612 is the same as the one in my case. That is, the > > sleeping thread may or may not hold a vnode lock depending on context. > I do not think that in-tree code sleeps with a vnode lock held in > the lf_advlock(). Otherwise, system would hang in lock cascade by > an attempt to obtain an advisory lock. I think we can even assert > this with witness. > There is another sleep, which Jilles mentioned, in lf_purgelocks(), > called from vgone(). This sleep indeed occurs under the vnode lock, and > as such must be non-suspendable. The sleep waits until other threads > leave the lf_advlock() for the reclaimed vnode, and they should leave in > deterministic time due to issued wakeups. So this sleep is exempt from > the considerations, and TDF_SBDRY there is correct. > I am fine with either the braces around sx_sleep() in lf_advlock() to > clear TDF_SBDRY (sigdeferstsop()), or with the latest patch I sent, > which adds temporal override for TDF_SBDRY with TDF_SRESTART. My > understanding is that you prefer the later. If I do not mis-represent > your position, I understand why you do prefer that. The TDF_SRESTART change does fix some more problems such as umount -f getting stuck in lf_purgelocks(). However, it introduces some subtle issues that may not necessarily be a sufficient objection. Firstly, adding this closes the door on fixing signal handling for fcntl(F_SETLKW). Per POSIX, any caught signal interrupts fcntl(F_SETLKW), even if SA_RESTART is set for the signal, and the Linux man page documents the same. Our man page has documented that SA_RESTART behaves normally with fcntl(F_SETLKW) since at least FreeBSD 2.0. This could normally be fixed via if (error == ERESTART) error = EINTR; but that is no longer possible if there are [ERESTART] errors that should still restart. Secondly, fcntl(F_SETLKW) restarting after a stop may actually be observable, contrary to what I wrote before. This is due to the fair queuing. Suppose thread A has locked byte 1 a while ago and thread B is trying to lock byte 1 and 2 right now. Then thread C will be able to lock byte 2 iff thread B has not blocked yet. If thread C will not be allowed to lock byte 2 and will block on it, the TDF_SRESTART change will cause it to be awakened if thread B is stopped. When thread B resumes, the region to be locked will be recomputed. This scenario unambiguously violates the POSIX requirement but I don't know how bad it is. Note that all these threads must be in separate processes because of fcntl locks' strange semantics. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: thread suspension when dumping core
On Tue, Jun 07, 2016 at 07:01:55PM +0300, Konstantin Belousov wrote: > On Tue, Jun 07, 2016 at 04:24:53PM +0200, Jilles Tjoelker wrote: > > On Tue, Jun 07, 2016 at 07:29:56AM +0300, Konstantin Belousov wrote: > > > This looks as if we should not ignore suspension requests in > > > thread_suspend_check() completely in TDF_SBDRY case, but return either > > > EINTR or ERESTART (most likely ERESTART). Note that the goal of > > > TDF_SBDRY is to avoid suspending in the protected region, not to make an > > > impression that the suspension does not occur at all. > > This looks like it would revert r246417 and re-introduce the bug fixed > > by it (unexpected [EINTR] and short reads/writes after stop signals). > Well, the patch returns ERESTART and not EINTR, so the syscall should > be retried after all the unwinding. That fixes the [EINTR] part of the problem but not short reads and writes. > > After r246417, TDF_SBDRY is intended for sleeps that occur while holding > > resources such as vnode locks and are normally short but should be > > interruptible by fatal signals because they may occasionally be > > indefinitely long (such as a non-responsive NFS server). > > It looks like yet another kind of sleep may be required, since advisory > > locks still hold some filesystem resources across the sleep (though not > > vnode locks). > I do not think that adv locks enter sleep with any resource held which > would block other threads. But I agree with the statement because the > lock might be granted and then the stopped thread would appear to own > the blocking resource. It does not hold any resource used by normal operations, but it blocks a forced unmount (umount -f hangs in [purgelocks] with tmpfs in a recent stable/10). If queuing is supposed to be fair, then granting the lock to the stopped thread is correct and aborting the sleep with [ERESTART] would break it. The kern_lockf.c code seems to go to some lengths to make queuing fair. This does not seem very important, though. Also, restarting a locking call violates some text in POSIX XSH's fcntl page that the range of bytes to be locked shall be determined before the thread blocks (this may be affected by the current seek offset and the file size). I don't know whether violating this will break any applications. The text has the problem that there is no way to distinguish between a thread that is in fcntl() and has not yet blocked and a thread that has blocked, even though it seems intuitively clear. > > We then have four kinds: > > * uninterruptible by signals, ignores stops (default) > > * interruptible by signals, ignores stops (current TDF_SBDRY with > > PCATCH) > > * interruptible by signals, freezes in place on stops (avoids > > unexpected short I/O) (current PCATCH, otherwise) > > * interruptible by signals, fails with [ERESTART] on stops (avoids > > holding resources across a stop) (new) > > The new kind of sleep would fail with [ERESTART] only for stops, since > > [EINTR] should only be returned if a signal handler was called. There > > cannot be a signal handler since a SIGTSTP/SIGTTIN/SIGTTOU signal with a > > handler does not stop the process. > And where would this new kind of sleep used ? The advlock sleep is the one > place. Does fifo sleep for reader or writer on open require this kind > of handling (IMO no) ? > I think this can be relatively easily implemented with either a flag > for XXXsleep(9) (my older style of PBDRY) or using only the thread flag > (jhb' newer TDF_SBDRY approach). Probably the later should be used, for > consistency and easier marking of larger blocks of code. In this case it is clear which sleep(9) calls should be affected so it may be better to avoid more hidden state. I also wonder whether we may be overengineering things here. Perhaps the advlock sleep can simply turn off TDF_SBDRY. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: thread suspension when dumping core
On Tue, Jun 07, 2016 at 07:29:56AM +0300, Konstantin Belousov wrote: > On Mon, Jun 06, 2016 at 09:17:41PM -0700, Mark Johnston wrote: > > Sure, see below. For reference: > > td_flags = 0xa84c = INMEM | SINTR | CANSWAP | ASTPENDING | SBDRY | > > NEEDSUSPCHK > > td_pflags = 0 > > td_inhibitors = 0x2 = SLEEPING > > td_locks = 0 > > stack: > > mi_switch+0x21e sleepq_catch_signals+0x377 sleepq_wait_sig+0xb _sleep+0x29d > > ... > > p_flag = 0x10080080 = INMEM | STOPPED_SINGLE | HADTHREADS > > p_flag2 = 0 > > The thread is sleeping interruptibly. The NEEDSUSPCHK flag is set, yet the > > SLEEPABORT flag is not, so the thread can not have been sleeping when > > thread_single() was called - else sleepq_abort() would have been > > invoked and set SLEEPABORT. We are at the second sleepq_switch() call in > > sleepq_catch_signals(), and no signal was pending, so we called > > thread_suspend_check(), which returned 0 because of SBDRY. So we went to > > sleep. > > I note that this couldn't have happened prior to r283320. That change > > was apparently motivated by a similar hang, but in that case the thread > > was suspended (with a vnode lock held) rather than asleep. It looks like > > our internal fix also added a change to set TDF_SBDRY around > > filesystem-specific syscalls, which often sleep interruptibly while > > holding vnode locks. But I don't think that's the problem here, as you > > noted with lf_advlock(). > > With r283320 reverted, P_STOPPED_SIG would not have been set, so > > thread_suspend_check() would have suspended us, allowing the core dump > > to proceed. I had thought that using SINGLE_BOUNDRY beforing coredumping > > would fix both hangs, but I guess that wouldn't help SINGLE_ALLPROC, so > > this is probably the wrong place to be solving the problem. > This looks as if we should not ignore suspension requests in > thread_suspend_check() completely in TDF_SBDRY case, but return either > EINTR or ERESTART (most likely ERESTART). Note that the goal of > TDF_SBDRY is to avoid suspending in the protected region, not to make an > impression that the suspension does not occur at all. This looks like it would revert r246417 and re-introduce the bug fixed by it (unexpected [EINTR] and short reads/writes after stop signals). After r246417, TDF_SBDRY is intended for sleeps that occur while holding resources such as vnode locks and are normally short but should be interruptible by fatal signals because they may occasionally be indefinitely long (such as a non-responsive NFS server). It looks like yet another kind of sleep may be required, since advisory locks still hold some filesystem resources across the sleep (though not vnode locks). We then have four kinds: * uninterruptible by signals, ignores stops (default) * interruptible by signals, ignores stops (current TDF_SBDRY with PCATCH) * interruptible by signals, freezes in place on stops (avoids unexpected short I/O) (current PCATCH, otherwise) * interruptible by signals, fails with [ERESTART] on stops (avoids holding resources across a stop) (new) The new kind of sleep would fail with [ERESTART] only for stops, since [EINTR] should only be returned if a signal handler was called. There cannot be a signal handler since a SIGTSTP/SIGTTIN/SIGTTOU signal with a handler does not stop the process. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: sdhci_pci.ko fails to load
On Sun, Mar 20, 2016 at 04:05:34PM -0600, Ian Lepore wrote: > On Sun, 2016-03-20 at 22:33 +0100, Guido Falsi wrote: > > On 03/20/16 22:21, Guido Falsi wrote: > > > On 03/20/16 22:18, Conrad Meyer wrote: > > > > Try 'kldload mmc' first. 'sdhci_pci' is missing a MODULE_DEPEND > > > > on mmc. > > > As I said, when loading sdhci_pci I had already loaded module mmc. > > > Anyway I'll try that again just to make sure, maybe I missed it and > > > thought I had it loaded. > > > I'll followup shortly. > > I confirm that I had already loaded mmc.ko. > [snip] > > the full error in dmesg is the same as stated before: > > link_elf_obj: symbol mmc_driver undefined > > linker_load_file: Unsupported file type > > Meybe the symbol is optimized out by the compiler in the module? > I suspect this is caused by my r292180 back in December. I'm trying to > figure out if that's the case and if so, how to fix it. I think this is caused by the missing MODULE_DEPEND. The kernel linker only looks for symbols in the ELF objects containing the module itself and its declared dependencies. If mmc is compiled into the main kernel image, this is always satisfied. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Possible problem with the ${name}_chdir variable behaviour in /etc/c.subr
On Sun, Mar 20, 2016 at 05:51:30PM +, Tim Preston wrote:
> I was having a problem making the audio/teamspeak3-server port start
> fro the rc.d scripts on a -current box that I’ve just built. I
> eventually traced this down to it not starting in the correct
> directory.
> Looking at the rc.d script I could see that it was attempting to chdir
> to the correct directory by setting the teamspeak_chdir variable. So
> it looked like this feature was misbehaving in some way.
> To test this I put together an rc.d script for pwd that used
> ${name}_chdir based on how the teamspeak rc.d script was setup.
> Running this on a 10.3 box showed that it was changing to the
> specified directory as expected, but on -current it was not.
> I’ve taken a quick look at rc.subr on both boxes but I can’t
> immediately see what’s breaking this, and I can’t see anything in the
> setup or behaviour of the -current box that would point to it being
> something that I’ve broken somehow there.
> I was wondering if anyone else could reproduce this?
The NAME_chdir variable prepends 'cd $NAME_chdir && ' to the command at
a certain point. Prepending further commands like limits (as added by
SVN r288291) will not work properly. The older $NAME_prepend feature is
broken in the same way. The cd command is having the limits or similar
applied to it while the real command is running without proper limits
and current directory.
The NAME_user and NAME_nice variables use sh -c to avoid this problem
but it introduces double-quoting issues (i.e. using arguments containing
certain special characters requires an additional unexpected level of
quoting).
Prepending cd at a later time would avoid the problem with limits and
NAME_prepend and allow getting rid of sh -c, but would change the
directory as root so permission problems with NAME_user would be
detected later.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: buffer overflow warning in /bin/sh
d/base/projects/clang380-import/bin/sh/parser.c:827:6 > #12 0x811c7c9 in parsecmd > /share/dim/src/freebsd/base/projects/clang380-import/bin/sh/parser.c:224:6 > #13 0x811046f in cmdloop > /share/dim/src/freebsd/base/projects/clang380-import/bin/sh/main.c:217:7 > #14 0x811015e in main > /share/dim/src/freebsd/base/projects/clang380-import/bin/sh/main.c:178:3 > #15 0x80557c9 in _start1 (/bin/sh+0x80557c9) > Address 0xbfbfe380 is located in stack of thread T0 at offset 32 in frame > #0 0x811e8ff in readtoken1 > /share/dim/src/freebsd/base/projects/clang380-import/bin/sh/parser.c:1400 > This frame has 3 object(s): > [16, 20) 'bqlist' > [32, 128) 'state_static' <== Memory access at offset 32 is inside this > variable > [160, 170) 'buf' > HINT: this may be a false positive if your program uses some custom stack > unwind mechanism or swapcontext > (longjmp and C++ exceptions *are* supported) > SUMMARY: AddressSanitizer: stack-buffer-overflow > /share/dim/src/freebsd/base/projects/clang380-import/bin/sh/parser.c:1419:22 > in readtoken1 > Shadow bytes around the buggy address: > 0x57f7fc20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fc30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fc40: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 00 00 > 0x57f7fc50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fc60: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 04 f2 > =>0x57f7fc70:[f3]f3 f3 f3 f3 f3 00 00 00 00 00 00 f2 f2 f2 f2 > 0x57f7fc80: 00 02 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fc90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fca0: 00 00 00 00 00 00 00 00 f1 f1 04 f2 04 f2 04 f2 > 0x57f7fcb0: 04 f2 04 f3 00 00 00 00 00 00 00 00 00 00 00 00 > 0x57f7fcc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Heap right redzone: fb > Freed heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack partial redzone: f4 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user:f7 > Container overflow: fc > Array cookie:ac > Intra object redzone:bb > ASan internal: fe > Left alloca redzone: ca > Right alloca redzone:cb > ==9912==ABORTING > This may be a false positive though. The reported store, which is near the top of the function, is clearly within bounds. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: Can't run `make universe` on universe11a.freebsd.org and ref11-amd64.freebsd.org (anymore); [shell] globbing is broken [there]
On Mon, Jan 04, 2016 at 12:08:43PM -0800, NGie Cooper wrote: > > It looks like a libc problem though, because I’ve run into this > > issue with both /bin/sh and bash (my default login shell). I’m not > > sure why this isn’t reproing on my VM (yet). > > This doesn’t repro in universe10a.freebsd.org (another jail on the > > same machine I think…). > > It was working yesterday on ref11-amd64.freebsd.org before the > > system was upgraded (it was running October sources), and wasn’t > > working on universe11a.freebsd.org yesterday (it was running > > December sources yesterday). > delphij@ pointed me in the right direction (thanks :)..). Globbing > expressions seems extremely broken with LANG set to en_US.UTF-8 [at > least]. > $ echo $LANG > en_US.UTF-8 > $ hostname > universe11a.freebsd.org > $ (unset LANG; cd sys/amd64/conf && echo [A-Z0-9]*[A-Z0-9]) > DEFAULTS GENERIC GENERIC-NODEBUG MINIMAL NOTES Traditionally, ranges in bracket expressions like A-Z meant characters that collate between A and Z, inclusive. Although this used to be in POSIX and is widely implemented, it does not make much sense. POSIX.1-2008 leaves ranges undefined in locales other than the POSIX locale. Therefore, it is an option to disable collation for ranges and just compare character codes. The problem started to occur more often with the new collation code, which supports UTF-8 and uses CLDR's different collation order, but strange results from [A-Z] can be encountered in much older versions. Bash behaves similarly to sh, but supports 'shopt -s globasciiranges' to disable collation. In some sense this is strange, an option that needs to be enabled to provide the behaviour most people expect. Alternatively, the pattern could be rewritten to be locale-sensitive: [[:upper:][:digit:]]*[[:upper:][:digit:]] -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: /bin/ls formatting broken for non-C(?) locales
On Fri, Nov 20, 2015 at 12:02:13PM +0100, Baptiste Daroussin wrote: > On Fri, Nov 20, 2015 at 11:42:53AM +0100, Baptiste Daroussin wrote: > > On Fri, Nov 20, 2015 at 11:05:56AM +0300, Sergey V. Dyatko wrote: > > > subj. http://i.imgur.com/F9QO29l.png > > > it is on head@r290573: > > > WTR: > > > env LC_ALL=uk_UA.UTF-8 ls -la /usr/ports/databases/ or env > > > LC_ALL=ru_RU.UTF-8 > > > ls -la /usr/ports/databases/ > > > env LC_ALL=C ls -la /usr/ports/databases/ works fine > > > also on old stable/10 (r286868) as I can see 'month' field length 3 > > > symbols > > Thanks for reporting, I can reproduce the issue with some other > > locales. The thing is there seems to be no standard for abbreviated > > length. Formerly we had a 3 character lenght for abbreviated month. > > We now use CLDR which seems to follow the abbreviated rules from IBM: > > "Each string must be of equal length and contain 5 characters or less" > > There are 2 possible fixes: either always pad those in the locale > > definition which seems wrong or modify ls so that it by itself pads > > properly. > > Neither posix nor ISO-14652 defines the length of the abbreviated form > > padding in the locales themself would be wrong so I do propose to > > pad in the ls command. And padding with 5 characters. > For the record glibc/linux had the same problem: > https://sourceware.org/bugzilla/show_bug.cgi?id=9859 > "fixed" in coreutils (gnu ls) the way I propose to do for us > http://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commit;h=612b647dd16d5abc03b295abe42d8b4a0fe660f7 Coreutils fixed it slightly better: it calculates the maximum width of the abbreviated month names and pads to that (with a maximum of 5). In particular, this ensures that the output does not change for locales that have 3-character abbreviations, such as the POSIX locale. I think this is valuable. They also keep the list of month names from this calculation and they say this speeds up ls noticeably compared to having strftime() expand %b for every file. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
Re: futimens and utimensat vs birthtime
On Sun, Aug 16, 2015 at 10:26:21PM +0800, Julian Elischer wrote: On 8/15/15 1:39 AM, John Baldwin wrote: On Friday, August 14, 2015 10:46:10 PM Julian Elischer wrote: I would like to implement this call. but would like input as to it's nature. The code inside the system would already appear to support handling three elements, though it needs some scrutiny, so all that is needed is a system call with the ability to set the birthtime directly. Whether it should take the form of the existing calls but expecting three items is up for discussion. Maybe teh addition of a flags argument to specify which items are present and which to set. ideas? I believe these should be new calls. Only utimensat() provides a flag argument, but it is reserved for AT_* flags. Using AT_* flags for things unrelated to pathnames is not without precedent: AT_REMOVEDIR for unlinkat() and AT_EACCESS for faccessat(). This isn't suitable for a large number of flags, though. I wasn't suggesting we keep the old ones and silently make them take 3 args :-) I was thining of suplementing them wth new syscalls and the obvious names are those you suggested. however I do wonder if there will ever be a need for a 4th... This could be indicated by yet another flag. I'm a bit disappointed that setting birthtimes apparently wasn't needed when I added futimens and utimensat. However, they are not part of any release yet, so it may be possible to remove them at some point. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: futimens and utimensat vs birthtime
On Fri, Aug 14, 2015 at 10:39:41AM -0700, John Baldwin wrote: On Friday, August 14, 2015 10:46:10 PM Julian Elischer wrote: I would like to implement this call. but would like input as to it's nature. The code inside the system would already appear to support handling three elements, though it needs some scrutiny, so all that is needed is a system call with the ability to set the birthtime directly. Whether it should take the form of the existing calls but expecting three items is up for discussion. Maybe teh addition of a flags argument to specify which items are present and which to set. ideas? I believe these should be new calls. Only utimensat() provides a flag argument, but it is reserved for AT_* flags. I would be fine with something like futimens3() and utimensat3() (where 3 means three timespecs). Jilles implemented futimens() and utimensat(), so he might have ideas as well. I would probably stick the birth time in the third (final) timespec slot to make it easier to update new code (you can use an #ifdef just around ts[2] without having to #ifdef the entire block). Without adding new syscalls, it is possible to use the first tv_nsec to indicate that a new birth time is present. In that case, times[0].tv_nsec == UTIME_WITHBIRTHTIME would indicate that times has 4 instead of 2 elements. Whether you want to do this instead of adding two more system calls is a different question. Also note that, in some sense, the inability to set the birthtime forward is a feature. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: FreeBSD_HEAD-tests - Build #1262 - Still Unstable
On Sun, Aug 09, 2015 at 08:08:21PM +, jenkins-ad...@freebsd.org wrote: FreeBSD_HEAD-tests - Build #1262 - Still Unstable: Build information: https://jenkins.FreeBSD.org/job/FreeBSD_HEAD-tests/1262/ Full change log: https://jenkins.FreeBSD.org/job/FreeBSD_HEAD-tests/1262/changes Full build log: https://jenkins.FreeBSD.org/job/FreeBSD_HEAD-tests/1262/console Change summaries: No changes The failed test cases: 2 tests failed. FAILED: lib.libc.locale.mbrtowc_test.mbrtowc_object Error Message: Invalid sequence FAILED: lib.libc.locale.mbstowcs_test.mbstowcs_basic Error Message: /builds/FreeBSD_HEAD/contrib/netbsd-tests/lib/libc/locale/t_mbstowcs.c:168: (ssize_t)mbstowcs(wbuf, t-data, SIZE-1): Illegal byte sequence SVN r286490 exposed the brokenness of these tests. Since RFC 3629 (November 2003), UTF-8 has been restricted to code points up to 0x10 (inclusive). The higher sequences should be removed from the tests (or, even better, be expected to cause [EILSEQ]). I can provide a patch, but thought I'd explain the problem first. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: mergemaster failing with read-only /usr/src
On Sun, May 03, 2015 at 02:03:49PM +0200, Wolfgang Zenker wrote: I'm trying to update this system: FreeBSD pomona 11.0-CURRENT FreeBSD 11.0-CURRENT #0: Mon Apr 13 03:48:04 CEST 2015 wolfgang@pomona:/usr/obj/usr/src/sys/UBQTERL mips Source for that was probably from about April 11th. I sucessfully built world and kernel, ran mergemaster -p and make installworld on rev 282299 but then mergemaster fails with: # mergemaster -iFU *** Creating the temporary root environment in /var/tmp/temproot *** /var/tmp/temproot ready for use *** Creating and populating directory structure in /var/tmp/temproot /bin/sh: cannot create routing_test.tmp: Read-only file system *** FATAL ERROR: Cannot 'cd' to /usr/src and install files to the temproot environment Filesystems are mounted like this: # mount /dev/da0s2a on / (ufs, local, noatime) devfs on /dev (devfs, local, multilabel) /dev/da0s1 on /boot (msdosfs, local) vulcan.lyx:/usr/src11 on /usr/src (nfs, read-only) vulcan.lyx:/var/obj/11/mips64 on /usr/obj (nfs) This used to work before. Any ideas, any further info I could provide? This broke after a test was added for etc/rc.d/. Without special code, this causes these tests to be built and installed as part of mergemaster/etcmerge, like other parts of etc. As a workaround you can do: echo make -C etc obj all | make buildenv on the build machine after make buildworld. Then mergemaster will work, even with a read-only /usr/obj. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: seekdir/readdir patch .. Call for Review.
On Fri, May 01, 2015 at 07:17:42PM +0300, Konstantin Belousov wrote: On Fri, May 01, 2015 at 03:04:51PM +0800, Julian Elischer wrote: if you are interested in readdir(3), seekdir(3) and telldir(3) then you should look at https://reviews.freebsd.org/D2410 this patches around a problem in seekdir() that breaks Samba. Seekdir(3) will not work as expected when files prior to the point of interest in directory have been deleted since the directory was opened. Windows clients using Samba cause both these things to happen, causing the next readdir(3) after the bad seekdir(3) to skip some entries and return the wrong file. Samba only needs to step back a single directory entry in the case where it reads an entry and then discovers it can't fit it into the buffer it is sending to the windows client. It turns out we can reliably cater to Samba's requirement because the last returned element is always still in memory, so with a little care, we can set our filepointer back to it safely. (once) seekdir and readdir (and telldir()) need a complete rewrite along with getdirentries() but that is more than a small edit like this. Can you explain your expectations from the whole readdir() vs. parallel directory modifications interaction ? From what I understood so far, there is unlocked modification of the container and parallel iterator over the same container. IMO, in such situation, whatever tweaks you apply to the iterator, it is still cannot be made reliable. Before making single-purpose changes to the libc readdir and seekdir code, or to the kernel code, it would be useful to state exact behaviour of the dirent machinery we want to see. No, 'make samba works in my situation' does not sound good enough. Consider the subsequence of entries that existed at opendir() time and were not removed until now. This subsequence is clearly defined and does not have concurrency problems. The order of this subsequence must remain unchanged and seekdir() must be correct with respect to this subsequence. Additionally, two other kinds of entries may be returned. New entries may be inserted anywhere in between the entries of the subsequence, and removed entries may be returned as if they were still part of the subsequence (so that not every readdir() needs a system call). A simple implementation for UFS-style directories is to store the offset in the directory (all bits of it, not masking off the lower 9 bits). This needs d_off or similar in struct dirent. The kernel getdirentries() then needs a similar loop as the old libc seekdir() to go from the start of the 512-byte directory block to the desired entry (since an entry may not exist at the stored offset within the directory block). This means that a UFS-style directory cannot be compacted (existing entries moved from higher to lower offsets to fill holes) while it is open for reading. An NFS exported directory is always open for reading. This also means that duplicate entries can only be returned if that particular filename was deleted and created again. Without kernel support, it is hard to get telldir/seekdir completely reliable. The current libc implementation is wrong since the holes within the block just disappear and change the offsets of the following entries; the kernel cannot fix this using entries with d_fileno = 0 since it cannot know, in the general case, how long the deleted entry was in the filesystem-independent dirent format. My previous idea of storing one d_fileno during telldir() is wrong since it will fail if that entry is deleted. If you do not care about memory usage (which probably is already excessive with the current libc implementation), you could store at telldir() time the offset of the current block returned by getdirentries() and the d_fileno of all entries already returned in the current block. The D2410 patch can conceptually work for what Samba needs, stepping back one directory entry. I will comment on it. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: readdir/telldir/seekdir problem (i think)
On Fri, Apr 24, 2015 at 04:28:12PM -0400, John Baldwin wrote: Yes, this isn't at all safe. There's no guarantee whatsoever that the offset on the directory fd that isn't something returned by getdirentries has any meaning. In particular, the size of the directory entry in a random filesystem might be a different size than the structure returned by getdirentries (since it converts things into a FS-independent format). This might work for UFS by accident, but this is probably why ZFS doesn't work. However, this might be properly fixed by the thing that ino64 is doing where each directory entry returned by getdirentries gives you a seek offset that you _can_ directly seek to (as opposed to seeking to the start of the block and then walking forward N entries until you get an inter-block entry that is the same). The ino64 branch only reserves space for d_off and does not use it in any way. This is appropriate since actually using d_off is a major feature addition. A proper d_off would still be useful even if UFS's readdir keeps masking off the offset so a directory read always starts at the beginning of a 512-byte directory block, since this allows more distinct offset values than safely using getdirentries()'s *basep. With d_off, one outer loop must read at least one directory block to avoid spinning indefinitely, while using getdirentries()'s *basep requires reading the whole getdirentries() buffer. Some Linux filesystems go further and provide a unique d_off for each entry. Another idea would be to store the last d_ino instead of dd_loc into the struct ddloc. On seekdir(), this would seek to loc_seek as before and skip entries until that d_ino is found, or to the start of the buffer if not found (and possibly return some entries again that should not be returned, but Samba copes with that). -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: SSE in libthr
On Fri, Mar 27, 2015 at 03:26:17PM -0400, Eric van Gyzen wrote: In a nutshell: Clang emits SSE instructions on amd64 in the common path of pthread_mutex_unlock. This reduces performance by a non-trivial amount. I'd like to disable SSE in libthr. How about saving and restoring the FPU/SSE state eagerly instead of the current CR0.TS-based lazy method? There is overhead associated with #NM exception handling (fpudna) which is not worth it if FPU/SSE are used often. This would apply to userland threads only; kernel threads normally do not use FPU/SSE and handle the FPU/SSE state manually if they do. There is performance improvement potential in using SSE for optimizing string functions, for example. Even a simple SSE2 strlen easily outperforms the already optimized lib/libc/string/strlen.c in a microbenchmark, and many other string functions are slow byte-at-a-time implementations. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: r279278 failed to build (yacc: maximum table size exceeded)
On Wed, Feb 25, 2015 at 12:11:29PM -0800, Garrett Cooper wrote: I was going to propose something a bit more radical — I can remove the BOOTSTRAPPING conditionals and simplify the code on 10-STABLE / 11-CURRENT. Maintaining BOOTSTRAPPING is error prone and it’s not saving much time in the long run in builds (it's taking longer to diagnose issues, test them, and commit fixes which will break at a later date). I’ve been bitten by this once because I don’t run ancient CURRENT/STABLE (r279198) and here are a couple follow up commits bumping tools versions in the past (e.g. r278975, r269662, etc). Just a thought. This may be appropriate for contributed code that will build on older FreeBSD versions without issues, but I don't like being forced to add (mostly untested) compatibility code with usage of recent libc features. For example, utilities like cp and touch currently use utimensat/futimens without #ifdef mess or extra code in libegacy. The strict BOOTSTRAPPING conditionals allow removing bootstrap tools eventually, when building from such old versions as to need them is no longer supported. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: svn commit: r267977 - head/bin/mv
On Fri, Jun 27, 2014 at 04:06:53PM -0700, Xin Li wrote:
[moving discussion to freebsd-current@]
On 06/27/14 15:23, Jilles Tjoelker wrote:
On Fri, Jun 27, 2014 at 07:57:54PM +, Xin LI wrote:
Author: delphij
Date: Fri Jun 27 19:57:54 2014
New Revision: 267977
URL: http://svnweb.freebsd.org/changeset/base/267977
Log:
Always set UF_ARCHIVE on target (because they are by definition new files
and should be archived) and ignore error when we can't set it (e.g. NFS).
Reviewed by: ken
MFC after: 2 weeks
Modified:
head/bin/mv/mv.c
Modified: head/bin/mv/mv.c
==
--- head/bin/mv/mv.c Fri Jun 27 19:50:30 2014(r267976)
+++ head/bin/mv/mv.c Fri Jun 27 19:57:54 2014(r267977)
@@ -337,8 +337,8 @@ err: if (unlink(to))
* on a file that we copied, i.e., that we didn't create.)
*/
errno = 0;
- if (fchflags(to_fd, sbp-st_flags))
- if (errno != EOPNOTSUPP || sbp-st_flags != 0)
+ if (fchflags(to_fd, sbp-st_flags | UF_ARCHIVE))
+ if (errno != EOPNOTSUPP || ((sbp-st_flags ~UF_ARCHIVE) != 0))
warn(%s: set flags (was: 0%07o), to, sbp-st_flags);
tval[0].tv_sec = sbp-st_atime;
The part ignoring failures to set UF_ARCHIVE is OK. However, it seems
inconsistent to set UF_ARCHIVE on a cross-filesystem mv of a single
file, but not on a cross-filesystem mv of a directory tree or a file
newly created via shell output redirection.
If UF_ARCHIVE is supposed to be set automatically, I think this should
be done in the kernel, like msdosfs already does. However, I'm not sure
this is actually a useful feature: backup programs are smarter than an
archive attribute these days.
The flag is supposed to be set automatically (as my understanding of the
ZFS portion of implementation).
OK, I did not know that ZFS set UF_ARCHIVE automatically. If so, blindly
copying st_flags like the old code did is indeed suboptimal.
However in order to implement that way, we will have to stat() the
target file (attached). Personally, I think this is a little bit
wasteful, but it would probably something that we have to do if we
implement a switch to turn off automatic UF_ARCHIVE behavior.
This seems better. For example, it avoids UF_ARCHIVE spreading around
randomly on UFS.
Nits: the errno = 0 assignment seems unnecessary, can not should be
cannot.
Index: bin/mv/mv.c
===
--- bin/mv/mv.c (revision 267984)
+++ bin/mv/mv.c (working copy)
@@ -278,6 +278,7 @@ fastcopy(const char *from, const char *to, struct
static char *bp = NULL;
mode_t oldmode;
int nread, from_fd, to_fd;
+ struct stat to_sb;
if ((from_fd = open(from, O_RDONLY, 0)) 0) {
warn(fastcopy: open() failed (from): %s, from);
@@ -329,6 +330,7 @@ err: if (unlink(to))
*/
preserve_fd_acls(from_fd, to_fd, from, to);
(void)close(from_fd);
+
/*
* XXX
* NFS doesn't support chflags; ignore errors unless there's reason
@@ -336,10 +338,19 @@ err:if (unlink(to))
* if the server supports flags and we were trying to *remove* flags
* on a file that we copied, i.e., that we didn't create.)
*/
- errno = 0;
- if (fchflags(to_fd, sbp-st_flags | UF_ARCHIVE))
- if (errno != EOPNOTSUPP || ((sbp-st_flags ~UF_ARCHIVE) != 0))
- warn(%s: set flags (was: 0%07o), to, sbp-st_flags);
+ if (fstat(to_fd, to_sb) == 0) {
+ if ((sbp-st_flags ~UF_ARCHIVE) !=
+ (to_sb.st_flags ~UF_ARCHIVE)) {
+ errno = 0;
+ if (fchflags(to_fd,
+ sbp-st_flags | (to_sb.st_flags UF_ARCHIVE)))
+ if (errno != EOPNOTSUPP ||
+ ((sbp-st_flags ~UF_ARCHIVE) != 0))
+ warn(%s: set flags (was: 0%07o),
+ to, sbp-st_flags);
+ }
+ } else
+ warn(%s: can not stat, to);
tval[0].tv_sec = sbp-st_atime;
tval[1].tv_sec = sbp-st_mtime;
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Turning TESTS on by default
On Fri, Jun 06, 2014 at 03:14:52PM -0400, Julio Merino wrote: TL;DR - I plan to turn the TESTS src.conf knob ON by default on Tuesday once I have been able to perform enough sanity-checks of the build and all of them pass. The impact of this is that the FreeBSD Test Suite (see tests(7)) will be built and installed by default under /usr/tests/ along with the private atf libraries and binaries. There should be no other changes and this should be transparent to everyone. If this happens to break the world in any way, we can trivially roll the change back to fix the fallout. Some details TESTS was never intended to be disabled by default. However, the original patches that were committed months ago related to this feature broke the build and the easiest way out (instead of reverting the commits) was to set the knob to disabled. Unfortunately, it stayed that way even after the discovered problems were fixed. I am confident enough now that we have ironed out all major issues that this might introduce, so it is about time to enable TESTS by default again in HEAD. The benefits of this are that 1) we allow end users (especially consumers of binary releases!) to run the tests out of the box, as it has always been intended; and 2) we will be able to run the official release builds through testing via Jenkins, instead of having to issue custom builds. Actual change: https://phabric.freebsd.org/D188 I will update this thread when the change is committed and/or with any updates. Please let me know if I'm missing anything. This is certainly useful, but please fix installworld from a read-only (e.g. NFS) /usr/obj first. I reported this a while ago in http://lists.freebsd.org/pipermail/freebsd-testing/2014-May/000384.html build Kyuafile.auto during buildworld, not installworld. This message includes patches, although they are a bit ugly. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: another Make (maybe) problem
On Sat, Apr 05, 2014 at 11:12:36AM -0400, Robert Huff wrote: While it's probably not related to the nominal issue, a deeper look at the full buildworld log found ten instances of this: make[3]: /usr/src/share/mk/bsd.subdir.mk line 89: warning: duplicate script for target all_subdir_libnv ignored make[3]: /usr/src/share/mk/bsd.subdir.mk line 89: warning: using previous script for all_subdir_libnv defined here make[3]: /usr/src/share/mk/bsd.subdir.mk line 89: warning: duplicate script for target all-man_subdir_libnv ignored make[3]: /usr/src/share/mk/bsd.subdir.mk line 89: warning: using previous script for all-man_subdir_libnv defined here [snip] While it doesn't stop the build process ... it would be nice to eliminate it as a possible contributing factor. 1) Is this a problem, for this or anything else? 2) Is there a way to fix it, short of a successful buildworld/installworld? (My version of /usr/src/share/mk/bsd.subdir.mk, r263778, has 118 lines/388 words/3377 characters.) These messages are harmless, but are fixed by r264167. The cause is pretty much expected: libnv was listed in SUBDIR twice. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Hello fdclose
On Tue, Mar 18, 2014 at 12:23:19AM +0100, Mariusz Zaborski wrote:
After our previous discuss [1] I prepare fdclosedir(3) function which
was committed by Pawel (cc'ed) in commit r254499.
A while ago I also prepare the fdclose function. Unfortunately, this
new function is a little bit more tricky then previous one. Can I ask
you for a review of this patch?
Does this patch allow perl to stop writing to FILE._file? As pointed out
in
http://lists.freebsd.org/pipermail/freebsd-current/2013-January/039024.html
perlio.c in the perl source contains a function
PerlIOStdio_invalidate_fileno() that should modify a FILE such that
fclose() does not close the file descriptor but still frees all memory
(Perl has already called fflush()). Although using fdclose() could solve
this without touching the internals of FILE, this will make perlio.c
uglier with even more #ifdefs.
[snip]
--- //depot/user/oshogbo/capsicum/lib/libc/stdio/Symbol.map 2013-06-28
08:51:28.0
+++ /home/oshogbo/p4/capsicum/lib/libc/stdio/Symbol.map 2013-06-28
08:51:28.0
@@ -156,6 +156,7 @@
putwc_l;
putwchar_l;
fmemopen;
+ fdclose;
open_memstream;
open_wmemstream;
};
This should be in the FBSD_1.4 namespace (which does not exist yet).
[snip]
--- //depot/user/oshogbo/capsicum/lib/libc/stdio/fclose.c 2013-06-28
08:51:28.0
+++ /home/oshogbo/p4/capsicum/lib/libc/stdio/fclose.c 2013-06-28
08:51:28.0
[snip]
+int
+fdclose(FILE *fp)
+{
+ int fd, r, err;
+
+ if (fp-_flags == 0) { /* not open! */
+ errno = EBADF;
+ return (EOF);
+ }
+
+ r = 0;
+ FLOCKFILE(fp);
+ fd = fp-_file;
+ if (fp-_close != __sclose) {
+ r = EOF;
+ errno = EOPNOTSUPP;
+ } else if (fd 0) {
+ r = EOF;
+ errno = EBADF;
+ }
+ if (r == EOF) {
+ err = errno;
+ (void)cleanfile(fp, true);
+ errno = err;
+ } else {
+ r = cleanfile(fp, false);
+ }
FUNLOCKFILE(fp);
+
+ return (r == 0 ? fd : r);
If a file descriptor would be returned but cleanfile() returns an error
(e.g. write error on flush), the file descriptor is not returned and not
closed.
I think that in cases where fdclose() would be used, it is essential
that the file descriptor is never closed. This means that the API needs
to be different so it can report a write error but still return a file
descriptor. One way to do this is to return the file descriptor by
reference. Another is to expect the application to call fileno() and not
return the descriptor from the new function.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: r263096 sparc64: casperd: Unable to receive message from client: Cannot allocate memory.
On Thu, Mar 13, 2014 at 05:14:56AM -0700, Anton Shterenlikht wrote:
FreeBSD 11.0-CURRENT #1 r263096
Is casperd started by default?
I haven't enabled it in /etc/rc.conf
I think I get this error when trying
to use ping, e.g.:
# ping localhost
Broken pipe
# ping freebsd.org
Broken pipe
#
Mar 13 12:08:48 casperd[1313]: [ERROR] (casperd) Unable to receive message
from client: Cannot allocate memory.
Mar 13 12:08:50 last message repeated 2 times
Mar 13 12:09:57 casperd[1313]: [ERROR] (casperd) Unable to receive message
from client: Cannot allocate memory.
What does this error mean?
It looks like a bug causes the big endian flag to be lost. As a
result, the bits are interpreted as little endian and an extremely large
allocation is attempted. Try this patch:
Index: lib/libnv/nvlist.c
===
--- lib/libnv/nvlist.c (revision 262358)
+++ lib/libnv/nvlist.c (working copy)
@@ -582,7 +582,7 @@ nvlist_check_header(struct nvlist_header *nvlhdrp)
errno = EINVAL;
return (false);
}
- if ((nvlhdrp-nvlh_flags = ~NV_FLAG_ALL_MASK) != 0) {
+ if ((nvlhdrp-nvlh_flags ~NV_FLAG_ALL_MASK) != 0) {
errno = EINVAL;
return (false);
}
On another note, I don't think the sends to casperd should be generating
SIGPIPE. Instead, the EPIPE error can be handled as normal, without
immediately terminating the process. I propose the following patch, with
a .Dd bump to the man page (to test it, the first patch should not be
applied):
Index: lib/libnv/msgio.c
===
--- lib/libnv/msgio.c (revision 262358)
+++ lib/libnv/msgio.c (working copy)
@@ -147,7 +147,7 @@ msg_send(int sock, const struct msghdr *msg)
for (;;) {
fd_wait(sock, false);
- if (sendmsg(sock, msg, 0) == -1) {
+ if (sendmsg(sock, msg, MSG_NOSIGNAL) == -1) {
if (errno == EINTR)
continue;
return (-1);
@@ -345,7 +345,7 @@ buf_send(int sock, void *buf, size_t size)
ptr = buf;
do {
fd_wait(sock, false);
- done = send(sock, ptr, size, 0);
+ done = send(sock, ptr, size, MSG_NOSIGNAL);
if (done == -1) {
if (errno == EINTR)
continue;
Index: lib/libnv/nv.3
===
--- lib/libnv/nv.3 (revision 262358)
+++ lib/libnv/nv.3 (working copy)
@@ -310,7 +310,9 @@ The
.Fn nvlist_send
function sends the given nvlist over the socket given by the
.Fa sock
-argument.
+argument,
+without generating
+.Dv SIGPIPE .
Note that nvlist that contains file descriptors can only be send over
.Xr unix 4
domain sockets.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Import of DragonFly Mail Agent
On Tue, Feb 25, 2014 at 11:30:56AM +0100, Baptiste Daroussin wrote: On Mon, Feb 24, 2014 at 11:50:10PM +0100, Jilles Tjoelker wrote: On Mon, Feb 24, 2014 at 07:01:54PM +0400, Slawa Olhovchenkov wrote: On Mon, Feb 24, 2014 at 03:30:14PM +0100, Baptiste Daroussin wrote: On Mon, Feb 24, 2014 at 06:17:37PM +0400, Slawa Olhovchenkov wrote: On Sun, Feb 23, 2014 at 10:11:56PM +0100, Baptiste Daroussin wrote: As some of you may have noticed, I have imorted a couple of days ago dma (DragonFly Mail Agent) in base. I have been asked to explain my motivation so here they are. What's about suid, security separations etc? What do you mean? dma is changing user as soon as possible, dma will be capsicumized, what else do you want as informations? sendmail (in the past) have same behaviour (run as root and chage user). This is some security risk. For many scenario change user is not simple (for example -- send file from local user A to local user B, file with permsion 0400). sendmail will be forced to change behaviour -- mailnull suid program for place mail into queue and root daemon for deliver to user. This is more complex. Can be dma avoid this way? I'm a bit disappointed that dma uses setuid/setgid binaries, although it is not a regression because sendmail also uses this Unix misfeature. To avoid the large attack surface of set*id binaries (the untrusted user can set many process parameters, pass strange file descriptors, send signals, etc), I think it is better to implement trusted submission differently. A privileged daemon (not necessarily running as root) can listen on a Unix domain socket and use getpeereid(3) to verify the credentials of the client. As long as $anyone locally can send emails, what is the point of checking getpeereid(3)? Checking getpeereid(3) is useful to provide a more reliable indication of which user account originated the message, for example on web hosting servers. For this, it is best if the smarthost authenticates dma so a user cannot bypass dma. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Import of DragonFly Mail Agent
On Mon, Feb 24, 2014 at 07:01:54PM +0400, Slawa Olhovchenkov wrote: On Mon, Feb 24, 2014 at 03:30:14PM +0100, Baptiste Daroussin wrote: On Mon, Feb 24, 2014 at 06:17:37PM +0400, Slawa Olhovchenkov wrote: On Sun, Feb 23, 2014 at 10:11:56PM +0100, Baptiste Daroussin wrote: As some of you may have noticed, I have imorted a couple of days ago dma (DragonFly Mail Agent) in base. I have been asked to explain my motivation so here they are. What's about suid, security separations etc? What do you mean? dma is changing user as soon as possible, dma will be capsicumized, what else do you want as informations? sendmail (in the past) have same behaviour (run as root and chage user). This is some security risk. For many scenario change user is not simple (for example -- send file from local user A to local user B, file with permsion 0400). sendmail will be forced to change behaviour -- mailnull suid program for place mail into queue and root daemon for deliver to user. This is more complex. Can be dma avoid this way? I'm a bit disappointed that dma uses setuid/setgid binaries, although it is not a regression because sendmail also uses this Unix misfeature. To avoid the large attack surface of set*id binaries (the untrusted user can set many process parameters, pass strange file descriptors, send signals, etc), I think it is better to implement trusted submission differently. A privileged daemon (not necessarily running as root) can listen on a Unix domain socket and use getpeereid(3) to verify the credentials of the client. Note that the largest gain with set*id binaries is obtained when the last set*id binary is removed; we are pretty far from that. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [CFT] bsdinstall and zfsboot enhancements
On Sun, Jan 05, 2014 at 04:04:03PM -0500, Nathan Whitehorn wrote:
On 12/01/13 07:34, Jilles Tjoelker wrote:
On Sat, Nov 30, 2013 at 04:36:18PM -0600, Nathan Whitehorn wrote:
This took much longer than I'd anticipated, but the patch to init is
attached. I chose not to make the changes to init rather than
getttyent() and friends in libc, which I am open to revisiting.
lib/libpam/modules/pam_securetty/pam_securetty.c calls getttynam(3) and
will not allow root login on a fake TTY that getttynam() does not
know. This module is enabled by default for the login service.
So it is probably better to patch libc rather than init.
OK, here's a revised patch. This one is shorter and works by introducing
an auto flag (ideas for names appreciated) that means on if the line
is an active console and off otherwise. Note that the behavior is now:
- ttys marked off stay off
- ttys marked on stay on
- ttys marked auto are enabled iff they are console devices
- ttys not present in /etc/ttys stay off
This behavior change is much easier to implement when doing it in libc
for various structural reasons and allows the terminal type, etc. to be
specified in the usual way.
Instead of auto you could use ifconsole.
This looks sensible to me.
As a preparatory patch, you could remove se_index and session_index from
init. They are only used to warn about a changed slot number in utmp(5)
which is irrelevant with utmpx. This noise warning would also appear
in most cases when changing from a fake console entry to a real line
in /etc/ttys. Also, if you do decide to fake ttys entries in init rather
than libc, the patch to init will be simpler.
With the new patch, this is indeed the case: no changes to init are
necessary at all. This does not change any behavior unless explicitly
requested in /etc/ttys, so unless there are any objections in the next
couple days, I will commit it.
I have some small remarks inline below.
I would like to remove se_index and session_index from init soon if it
does not conflict with other work.
Index: include/ttyent.h
===
--- include/ttyent.h (revision 260331)
+++ include/ttyent.h (working copy)
@@ -37,6 +37,7 @@
#define _TTYS_OFF off
#define _TTYS_ONon
+#define _TTYS_AUTO auto
#define _TTYS_SECUREsecure
#define _TTYS_INSECURE insecure
#define _TTYS_WINDOWwindow
Index: lib/libc/gen/getttyent.c
===
--- lib/libc/gen/getttyent.c (revision 260331)
+++ lib/libc/gen/getttyent.c (working copy)
@@ -39,6 +39,9 @@
#include ctype.h
#include string.h
+#include sys/types.h
+#include sys/sysctl.h
+
static char zapchar;
static FILE *tf;
static size_t lbsize;
@@ -64,6 +67,32 @@
return (t);
}
+static int
+auto_tty_status(const char *ty_name)
+{
+ size_t len;
+ char *buf, *cons, *nextcons;
+
+ /* Check if this is an enabled kernel console line */
+ buf = NULL;
+ if (sysctlbyname(kern.console, NULL, len, NULL, 0) == -1)
+ return (0); /* Errors mean don't enable */
+ buf = malloc(len);
+ if (sysctlbyname(kern.console, buf, len, NULL, 0) == -1)
+ return (0);
conscontrol also does this, but is it possible that the length increases
between the checks?
+
+ if ((cons = strchr(buf, '/')) == NULL)
+ return (0);
+ *cons = '\0';
+ nextcons = buf;
+ while ((cons = strsep(nextcons, ,)) != NULL strlen(cons) != 0) {
+ if (strcmp(cons, ty_name) == 0)
+ return (TTY_ON);
+ }
+
+ return (0);
+}
+
struct ttyent *
getttyent(void)
{
@@ -126,6 +155,8 @@
tty.ty_status = ~TTY_ON;
else if (scmp(_TTYS_ON))
tty.ty_status |= TTY_ON;
+ else if (scmp(_TTYS_AUTO))
+ tty.ty_status |= auto_tty_status(tty.ty_name);
else if (scmp(_TTYS_SECURE))
tty.ty_status |= TTY_SECURE;
else if (scmp(_TTYS_INSECURE))
Index: libexec/getty/ttys.5
===
--- libexec/getty/ttys.5 (revision 260331)
+++ libexec/getty/ttys.5 (working copy)
@@ -102,8 +102,11 @@
.Pp
As flag values, the strings ``on'' and ``off'' specify that
.Xr init 8
-should (should not) execute the command given in the second field,
-while ``secure'' (if ``on'' is also specified) allows users with a
+should (should not) execute the command given in the second field.
+``auto'' will cause this line to be enabled if and only if it is
+an active kernel console device (it is equivalent to ``on'' in this
+case).
+The flag ``secure'' (if ``on'' is also specified) allows users with a
Please change if ``on'' is also specified to something like
Re: [CFT] bsdinstall and zfsboot enhancements
On Sat, Nov 30, 2013 at 04:36:18PM -0600, Nathan Whitehorn wrote: This took much longer than I'd anticipated, but the patch to init is attached. I chose not to make the changes to init rather than getttyent() and friends in libc, which I am open to revisiting. lib/libpam/modules/pam_securetty/pam_securetty.c calls getttynam(3) and will not allow root login on a fake TTY that getttynam() does not know. This module is enabled by default for the login service. So it is probably better to patch libc rather than init. The behavior changes are as follows: If the console device in /etc/ttys in marked on, instead of opening /dev/console, init will loop through the active kernel console devices, and for each will: 1. If the kernel console device is in /etc/ttys and marked on, it already has a terminal and will be ignored. 2. If marked off, that is an explicit statement that a console is not wanted and so it will be ignored. 3. If not present in /etc/ttys, init will run getty with whatever parameters console has. This seems to make sense. (3) is the main behavioral change. No changes in behavior will occur if /etc/ttys is not modified. If we turn on console by default, it will usually have no effect instead of trying to run multiple gettys, which is new. If we then also comment out the ttyu0 line, instead of marking it off, the result will be the conditional presence of a login prompt on the first serial port depending on whether it is an active console device for the kernel. I believe this is the behavior we are going for. The terminal type for the console entry should probably be changed to something other than unknown to reduce annoyance. Comments and test results would be appreciated. As a preparatory patch, you could remove se_index and session_index from init. They are only used to warn about a changed slot number in utmp(5) which is irrelevant with utmpx. This noise warning would also appear in most cases when changing from a fake console entry to a real line in /etc/ttys. Also, if you do decide to fake ttys entries in init rather than libc, the patch to init will be simpler. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] contrib/groff Queisce -Wdangling else
On Sat, Oct 26, 2013 at 11:04:29PM +0200, d...@gmx.com wrote:
Sean Bruno wrote, On 10/26/2013 17:04:
Index: contrib/groff/src/roff/troff/node.cpp
===
--- contrib/groff/src/roff/troff/node.cpp (revision 257159)
+++ contrib/groff/src/roff/troff/node.cpp (working copy)
@@ -4600,17 +4600,18 @@
}
else {
hunits rem = x - w*i;
-if (rem H0)
+if (rem H0) {
if (n-overlaps_horizontally()) {
if (out-is_on())
n-tprint(out);
out-right(rem - w);
+ } else {
+ out-right(rem);
}
- else
- out-right(rem);
while (--i = 0)
if (out-is_on())
n-tprint(out);
+}
}
}
There is no(intended) functional change.
This part indeed looks wrong. The while loop was not under the if (rem
H0) but now is. The closing brace should be added before instead of
after the while loop.
Also, putting braces around out-right(rem); is not needed.
I recommend making sure the object files do not change due to patches
like these.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [Heads Up] RCS removed from base
On Sun, Oct 06, 2013 at 10:43:21PM -0400, Eitan Adler wrote: RCS was removed from the base system in r256095. If you still want to use RCS please install either devel/rcs or devel/rcs57. If not, be sure to check out the alternatives (pun stolen and intended). Thanks for removing this piece of old GPL software. Perhaps a rewrite of ident(1) should be added, since Subversion still uses this keyword syntax and does not provide a utility to print keywords from a file that is not part of a Subversion checkout. This could be a shell script based on a command like strings $f | sed -n -e 's/^.*\(\$[[:alpha:]]*[[:alpha:]]: [^$]*\$\).*$/\1/p' or a C program (like what(1)). Likewise, merge(1) can be useful without RCS itself. However, consensus across operating system integrators appears to be that this functionality is best built into version control systems and not provided separately. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: installworld broken - osreldate.h: permission denied
On Sun, Sep 29, 2013 at 10:13:29AM +0200, Joel Dahl wrote: On Sat, Sep 28, 2013 at 11:19:51AM -0600, Ian Lepore wrote: On Sat, 2013-09-28 at 15:09 +0200, Joel Dahl wrote: Fresh HEAD. installworld from read-only /usr/obj and /usr/src: /usr/src/include/iconv.h osreldate.h /usr/include install: osreldate.h: Permission denied *** Error code 71 Stop. make[4]: stopped in /usr/src/include *** Error code 1 Everything was working fine 2 weeks ago, so it's a recent breakage. Okay, I just accidentally created conditions for this error on my system... I checked in a change to newvers.sh while a buildworld was running, which led to a situation where newvers.sh was newer than osreldate.h at the end of the buildworld. Then an installworld tried to regenerate osreldate.h due to its dependency on newvers.sh, which would fail if the obj was readonly at that point. I think we could see if something similar applies for you if you use this command: make -dm installworld SUBDIR_OVERRIDE=include I tried this with a fresh HEAD but the error message is still the same. /usr/src and /usr/obj are NFS mounted, FYI. I had the same problem as Joel. It has nothing to do with timestamps, but with the default -maproot -2:-2. The include/mk-osreldate.sh script creates osreldate.h from mktemp(1), so with mode 600. The squashed root (nobody) is then not allowed to read it. The below patch should fix it. Index: include/mk-osreldate.sh === --- include/mk-osreldate.sh (revision 255946) +++ include/mk-osreldate.sh (working copy) @@ -48,4 +48,5 @@ #define __FreeBSD_version $RELDATE #endif EOF +chmod 644 $tmpfile mv $tmpfile osreldate.h -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: restarting SYSCALL system call on amd64 loses arguments
On Tue, Sep 24, 2013 at 12:37:30AM +0300, Konstantin Belousov wrote: On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote: Has anyone taken a look at this PR yet? http://www.freebsd.org/cgi/query-pr.cgi?pr=182161 This looks like a valid bug, but probably not a valid testcase. Let me elaborate. When a signal is delivered, return from the signal handler is performed by the sigreturn(2), which reloads the whole register file when crossing kernel-user boundary due to sys_sigreturn(9) setting PCB_FULL_IRET flag. As result, the whole trap frame at the time of the syscall entry is restored, and ERESTART return is not exercised. I was not able to reproduce the issue with the supplied test program on HEAD. I suspect that the program actually exposed the bug in the signal delivery in the threaded processes, which I introduced for 9.1 and fixed in r251047 r251365. The ERESTART return happens if there is no signal or no longer a signal. The latter is how the bug in the PR occurs: a SIGCHLD delivery via handler in one thread races with a SIGCHLD acceptance in wait4() in another thread. Note wait4() returning a value in the other thread in the fourth line of the kdump output in the PR. For some reason, I can reproduce this easily on my local quad-core r255729 stable/9 system but not on ref9-amd64.freebsd.org or ref10-amd64.freebsd.org. I can also reproduce the bug on my local system by racing signal delivery via handler with acceptance in sigtimedwait(). -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: restarting SYSCALL system call on amd64 loses arguments
On Tue, Sep 24, 2013 at 10:29:09PM +0300, Konstantin Belousov wrote: On Tue, Sep 24, 2013 at 09:19:49PM +0200, Jilles Tjoelker wrote: On Tue, Sep 24, 2013 at 12:37:30AM +0300, Konstantin Belousov wrote: On Mon, Sep 23, 2013 at 10:26:13PM +0200, Tijl Coosemans wrote: Has anyone taken a look at this PR yet? http://www.freebsd.org/cgi/query-pr.cgi?pr=182161 This looks like a valid bug, but probably not a valid testcase. Let me elaborate. When a signal is delivered, return from the signal handler is performed by the sigreturn(2), which reloads the whole register file when crossing kernel-user boundary due to sys_sigreturn(9) setting PCB_FULL_IRET flag. As result, the whole trap frame at the time of the syscall entry is restored, and ERESTART return is not exercised. I was not able to reproduce the issue with the supplied test program on HEAD. I suspect that the program actually exposed the bug in the signal delivery in the threaded processes, which I introduced for 9.1 and fixed in r251047 r251365. The ERESTART return happens if there is no signal or no longer a signal. The latter is how the bug in the PR occurs: a SIGCHLD delivery via handler in one thread races with a SIGCHLD acceptance in wait4() in another thread. Note wait4() returning a value in the other thread in the fourth line of the kdump output in the PR. For some reason, I can reproduce this easily on my local quad-core r255729 stable/9 system but not on ref9-amd64.freebsd.org or ref10-amd64.freebsd.org. I can also reproduce the bug on my local system by racing signal delivery via handler with acceptance in sigtimedwait(). So, could you, please, check the r255844 on your machine ? I cannot reproduce it with that (patch applied to stable/9 kernel). The test programs run fine for minutes. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: I386 jail on amd64 CURRENT core dump in libc?
On Wed, Sep 04, 2013 at 12:14:03AM +0300, Konstantin Belousov wrote:
On Tue, Sep 03, 2013 at 08:25:29PM +0300, Vitalij Satanivskij wrote:
KB
KB Your installed libraries do not have proper debugging symbols.
KB Since the issue seems to be in the compat32 layer, you may try to start
KB with taking the ktrace of the failing program and see what syscall
failed,
KB if any.
For me problem gone after disabling
options CAPABILITY_MODE # Capsicum capability mode
options CAPABILITIES# Capsicum capabilities
in kernel conf
I'm found it when roll backing system to previos revisions.
On r254268 uniq inside i386 jail say that = unable to limit rights for
So I decide to check without Capsicum features...
Then the ktrace output would be esp. useful. Anyway, this is probably
cap_rights_limit(2) compat32 issue. Pawel may know more.
cap_rights_limit(2) should have been fixed in r254491, so options
CAPABILITIES should be OK (I have not tested such a kernel though).
However, capability mode does not work with compat32. There is no
sys/compat32/capabilities.conf (also, such a file would be poorly
maintainable), and therefore capability mode does not permit any
compat32 system calls. As a result, a compat32 capability mode process
crashes after failing to invoke sys_exit.
The below patch ('make sysent' should be run in sys/compat/freebsd32
after patching) makes the kernel admit that it does not support
capability mode for compat32. This does not help if a 64-bit binary
enters capability mode and then executes a 32-bit binary using
fexecve(2) but otherwise it helps. It makes compat32 dhclient and uniq
work again, albeit without Capsicum security enhancements.
Making capability mode work for compat32 binaries would be better but if
it is not possible for 10.0 then something like this patch should be
committed.
Index: sys/compat/freebsd32/freebsd32_capability.c
===
--- sys/compat/freebsd32/freebsd32_capability.c (revision 255093)
+++ sys/compat/freebsd32/freebsd32_capability.c (working copy)
@@ -50,6 +50,18 @@
MALLOC_DECLARE(M_FILECAPS);
int
+freebsd32_cap_enter(struct thread *td,
+struct freebsd32_cap_enter_args *uap)
+{
+
+ /*
+* We do not have an equivalent of capabilities.conf for freebsd32
+* compatibility, so do not allow capability mode for now.
+*/
+ return (ENOSYS);
+}
+
+int
freebsd32_cap_rights_limit(struct thread *td,
struct freebsd32_cap_rights_limit_args *uap)
{
@@ -148,6 +160,14 @@
#else /* !CAPABILITIES */
int
+freebsd32_cap_enter(struct thread *td,
+struct freebsd32_cap_enter_args *uap)
+{
+
+ return (ENOSYS);
+}
+
+int
freebsd32_cap_rights_limit(struct thread *td,
struct freebsd32_cap_rights_limit_args *uap)
{
Index: sys/compat/freebsd32/syscalls.master
===
--- sys/compat/freebsd32/syscalls.master(revision 255093)
+++ sys/compat/freebsd32/syscalls.master(working copy)
@@ -973,7 +973,7 @@
514AUE_CAP_NEW NOPROTO { int cap_new(int fd, uint64_t rights); }
515AUE_CAP_RIGHTS_GET NOPROTO { int cap_rights_get(int fd, \
uint64_t *rightsp); }
-516AUE_CAP_ENTER NOPROTO { int cap_enter(void); }
+516AUE_CAP_ENTER STD { int freebsd32_cap_enter(void); }
517AUE_CAP_GETMODE NOPROTO { int cap_getmode(u_int *modep); }
518AUE_PDFORK NOPROTO { int pdfork(int *fdp, int flags); }
519AUE_PDKILL NOPROTO { int pdkill(int fd, int signum); }
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Ports with daemons on uninstall...
On Sun, Jul 14, 2013 at 05:52:37PM +0200, Ian FREISLICH wrote: I have to ask if there's a standard for the way ports should handle their daemons when the port is uninstalled. I've encountered 3 varients of ports behaviour on uninstall: 1. Do nothing 2. Stop the daemon 3. Ask if the daemon should be stopped #1 closely followed by #3 are the least irritating when it comes to portupgrade because you can at least have the service running while upgrading. At least with #3 the upgrade gets paused until the propmpt is answered and you're then aware that some service will go away immediately so you can be prepared to restart it. #2 is extremely irritating because upgrading with portupgrade etc kills the service. For instance isc-dhcpd* does this which means that for some time, dhcp may be unavailable. It could be less irritating if it would automatically start the service, but that can have its own problems. Does the project have a preferred method for handling running daenmons on uninstall? I know that Linux will even start daemons on install. I think that almost all of this per-port code should be removed with pkgng. The HANDLE_RC_SCRIPTS pkg.conf option will stop/start the rc.d script during deinstallation/installation. By default, services are left running. Stopping the service on deinstall but not starting it again on install seems like a particularly bad idea. Apart from the annoyance of the restarts, automatic stopping and starting is probably the best policy for having things just work. Some daemons will crash or otherwise stop being useful when their files have been deleted or replaced, and the new rc.d script might be unable to stop the old daemon. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: rebuilding world fail
On Tue, Jun 04, 2013 at 07:39:59PM +0200, Rodrigo OSORIO wrote: Le 04/06/2013 18:08, Rodrigo OSORIO a écrit : // Rebuilding world for current, I found two blocking errors: // make installkernel retrun 1: Invalid argument // and when I try to execute mergemaster, the system // claims he's out of file descriptors. Other commands // also fail with the same message. // I have no clues about this issue, and the system log // didn't help. What are values of sysctl kern.openfiles and kern.maxfiles ? Have a look with fstat for this problem of file descriptors. After reboot, cause my system was locked kern.openfiles = 42 and kern.maxfiles = 15258. Now I move with my pending work on 10, but I think I can reproduce the behavior later, and post the live values. One other possibility is that the documented procedure was not followed and a new userland is being run on an old kernel. Since fairly recently, /bin/sh has used fcntl(F_DUPFD_CLOEXEC) which is not in 9.1-RELEASE although it has been in head for the better part of a year. Note that fdlopen() (possibly used by PAM) has depended on F_DUPFD_CLOEXEC for longer. You can try booting the new kernel manually or getting a /bin/sh binary from before svn r250267 from somewhere. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: FreeBSD-HEAD gets stuck on vnode operations
on
vnode_free_list_mtx to finish it's work and release v_interlock.
---
sys/kern/vfs_subr.c | 10 +-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/sys/kern/vfs_subr.c b/sys/kern/vfs_subr.c
index 0da6764..597f4b7 100644
--- a/sys/kern/vfs_subr.c
+++ b/sys/kern/vfs_subr.c
@@ -4703,7 +4703,15 @@ restart:
if (mp_ncpus == 1 || should_yield()) {
TAILQ_INSERT_BEFORE(vp, *mvp, v_actfreelist);
mtx_unlock(vnode_free_list_mtx);
- kern_yield(PRI_USER);
+ /*
+ * There is another thread owning the
+ * v_interlock mutex and possibly waiting on
+ * vnode_free_list_mtx, so pause in order for
+ * it to acquire the vnode_free_list_mtx
+ * mutex and finish the work, releasing
+ * v_interlock when finished.
+ */
+ pause(vi_lock, 1);
mtx_lock(vnode_free_list_mtx);
goto restart;
}
--
1.7.7.5 (Apple Git-26)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: FreeBSD-HEAD gets stuck on vnode operations
On Sun, May 26, 2013 at 10:52:07PM +0200, Roger Pau Monné wrote: On 26/05/13 22:20, Jilles Tjoelker wrote: Instead of a pause() that may be too short or too long, how about waiting for the necessary lock? In other words, replace the kern_yield() call with VI_LOCK(vp); VI_UNLOCK(vp);. This is also the usual approach to acquire two locks without imposing an order between them. Since there might be more than one locked vnode, waiting on a specific locked vnode seemed rather arbitrary, but I agree that the pause is also rather arbitrary. Also, can we be sure that the v_interlock mutex will not be destroyed while the syncer process is waiting for it to be unlocked? I think this is a major problem. My idea was too easy and will not work. That said, the code in mnt_vnode_next_active() appears to implement some sort of adaptive spinning for SMP. It tries VI_TRYLOCK for 200ms (default value of hogticks) and then yields. This is far too long for a mutex lock and if it takes that long it means that either the thread owning the lock is blocked by us somehow or someone is abusing a mutex to work like a sleepable lock such as by spinning or DELAY. Given that it has been spinning for 200ms, it is not so bad to pause for one additional microsecond. The adaptive spinning was added fairly recently, so apparently it happens fairly frequently that VI_TRYLOCK fails transiently. Unfortunately, the real adaptive spinning code cannot be used because it will spin forever as long as the thread owning v_interlock is running, including when that is because it is spinning for vnode_free_list_mtx. Perhaps we can try to VI_TRYLOCK a certain number of times. It is also possible to check the contested bit of vnode_free_list_mtx (sys/netgraph/netflow/netflow.c does something similar) and stop spinning in that case. A cpu_spinwait() invocation should also be added to the spin loop. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: rc.subr: disabling globbing while processing devfs rules
On Mon, Apr 01, 2013 at 02:06:50PM -0400, John Baldwin wrote:
Why not use 'local -' instead of the $- magic? That is:
devfs_rulesets_from_file()
{
local file _err _me -
...
set -f
...
}
That would seem to be simpler.
I had mentioned this possibility on IRC, but this feature is specific to
Almquist-derived shells (ash) and so something more portable was
selected. (It's still not standard because POSIX does not specify
local but it works on most shells in use.)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: r247839: broken pipe - for top, sudo and ports
On Thu, Mar 07, 2013 at 04:54:01AM -0100, Jan Beich wrote:
Jilles Tjoelker jil...@stack.nl writes:
On Tue, Mar 05, 2013 at 08:59:09PM +0100, Hartmann, O. wrote:
A truss top reveals this, is this of help?
[...]
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
socket(PF_LOCAL,SOCK_STREAM,0) = 4 (0x4)
connect(4,{ AF_UNIX /var/run/nscd },15)= 0 (0x0)
fcntl(4,F_SETFL,O_NONBLOCK) = 0 (0x0)
kqueue(0x80183b000,0x80122fc58,0x10,0x80062b308,0x80183b010,0x2) = 5 (0x5)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kqueue(0x5,0x7fffd2e0,0x1,0x0,0x0,0x0) = 6 (0x6)
kevent(6,{0x4,EVFILT_READ,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,1,0x4,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,0x0,0,{0x4,EVFILT_WRITE,EV_EOF,0,0x2000,0x0},1,0x0) = 1 (0x1)
sendmsg(0x4,0x7fffd290,0x0,0x1,0x1,0x0) ERR#32 'Broken pipe'
SIGNAL 13 (SIGPIPE)
process exit, rval = 0
Apparently there is a bug that causes nscd to close the connection
immediately but even then it is wrong that this terminates the calling
program with SIGPIPE.
The below patch prevents the SIGPIPE but cannot revive the connection to
nscd. This may cause numeric UIDs in top or increase the load on the
directory server. It is compile tested only.
[...]
The patch seems to fix the issue in a world after r247804. I don't see
numeric UIDs in top but without the patch top crashes with SIGPIPE a lot
less frequently than sudo or make install (in base/ports) for me.
In my case shutting down nscd helped, too. Compared to stock
nsswitch.conf I only have cache added.
Can you find what causes nscd to close the connection quickly, such as
using ktrace?
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: r247839: broken pipe - for top, sudo and ports
On Sun, Mar 10, 2013 at 08:26:03PM -0200, Jan Beich wrote:
Jilles Tjoelker jil...@stack.nl writes:
On Thu, Mar 07, 2013 at 04:54:01AM -0100, Jan Beich wrote:
Jilles Tjoelker jil...@stack.nl writes:
On Tue, Mar 05, 2013 at 08:59:09PM +0100, Hartmann, O. wrote:
A truss top reveals this, is this of help?
[...]
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
socket(PF_LOCAL,SOCK_STREAM,0) = 4 (0x4)
connect(4,{ AF_UNIX /var/run/nscd },15)= 0 (0x0)
fcntl(4,F_SETFL,O_NONBLOCK) = 0 (0x0)
kqueue(0x80183b000,0x80122fc58,0x10,0x80062b308,0x80183b010,0x2)
= 5 (0x5)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kqueue(0x5,0x7fffd2e0,0x1,0x0,0x0,0x0) = 6 (0x6)
kevent(6,{0x4,EVFILT_READ,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,1,0x4,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,0x0,0,{0x4,EVFILT_WRITE,EV_EOF,0,0x2000,0x0},1,0x0) = 1 (0x1)
sendmsg(0x4,0x7fffd290,0x0,0x1,0x1,0x0) ERR#32 'Broken pipe'
SIGNAL 13 (SIGPIPE)
process exit, rval = 0
Apparently there is a bug that causes nscd to close the connection
immediately but even then it is wrong that this terminates the calling
program with SIGPIPE.
The below patch prevents the SIGPIPE but cannot revive the connection to
nscd. This may cause numeric UIDs in top or increase the load on the
directory server. It is compile tested only.
[...]
The patch seems to fix the issue in a world after r247804. I don't see
numeric UIDs in top but without the patch top crashes with SIGPIPE a lot
less frequently than sudo or make install (in base/ports) for me.
In my case shutting down nscd helped, too. Compared to stock
nsswitch.conf I only have cache added.
Can you find what causes nscd to close the connection quickly, such as
using ktrace?
# single user mode
$ ktrace -p $(pgrep nscd); top -b; ktrace -c; kdump
71 nscd GIO fd 5 wrote 0 bytes
71 nscd GIO fd 5 read 32 bytes
0x 0400 1000 0100
|..|
0x0012 |..|
71 nscd RET kevent 1
71 nscd CALL accept(0x4,0,0)
71 nscd RET accept 6
We are in usr.sbin/nscd/nscd.c accept_connection() here.
71 nscd CALL getsockopt(0x6,0,0x1,0x7f9fce28,0x7f9fce24)
71 nscd RET getsockopt 0
Probably getpeereid().
On another note, nscd leaks the file descriptor if this, the below
init_query_state() or the below kevent() fails.
71 nscd CALL kevent(0x5,0x7f9fcf00,0x2,0,0,0x7f9fcf50)
71 nscd GIO fd 5 wrote 64 bytes
0x 0600 f9ff 1100 401f
|@.|
0x0012 401f 40e6 4002 0800 0600
|..@...@.@.|
0x0024 1100 0100 0400
|..|
0x0036 40e6 4002 0800 |..@.@.|
Adding an EVFILT_TIMER and an EVFILT_READ.
The data field for the EVFILT_TIMER is a bit strange. I would expect
0x1f40 (8000 decimal) but it puts instead
0x1f401f40. This does not happen when I run
tools/regression/kqueue/kqtest on a stable/9 amd64 machine or on
ref10-amd64 which currently runs r247722.
On a head (r248047) i386 machine, the data field looks right.
71 nscd GIO fd 5 read 0 bytes
71 nscd RET kevent 0
71 nscd CALL kevent(0x5,0x7f9fcec0,0x1,0,0,0x7f9fcee0)
71 nscd GIO fd 5 wrote 32 bytes
0x 0400 1100
|..|
0x0012 |..|
Probably registering interest for the next connection.
71 nscd GIO fd 5 read 0 bytes
71 nscd RET kevent 0
71 nscd CALL kevent(0x5,0,0,0x7f9fcec0,0x1,0)
71 nscd GIO fd 5 wrote 0 bytes
71 nscd GIO fd 5 read 32 bytes
0x 0600 f9ff 3000 0100
|..0...|
0x0012 40e6 4002 0800 |..@.@.|
The timer has already expired. This cannot be right. (It cannot be that
EVFILT_READ is broken and eight seconds actually passed because the send
calls would have worked in that case.)
tools/regression/kqueue/kqtest works correctly on the aforementioned
Re: r247839: broken pipe - for top, sudo and ports
On Tue, Mar 05, 2013 at 08:59:09PM +0100, Hartmann, O. wrote:
A truss top reveals this, is this of help?
[...]
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
stat(/etc/nsswitch.conf,{ mode=-rw-r--r--
,inode=162310,size=1007,blksize=32768 }) = 0 (0x0)
socket(PF_LOCAL,SOCK_STREAM,0) = 4 (0x4)
connect(4,{ AF_UNIX /var/run/nscd },15)= 0 (0x0)
fcntl(4,F_SETFL,O_NONBLOCK) = 0 (0x0)
kqueue(0x80183b000,0x80122fc58,0x10,0x80062b308,0x80183b010,0x2) = 5 (0x5)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kqueue(0x5,0x7fffd2e0,0x1,0x0,0x0,0x0) = 6 (0x6)
kevent(6,{0x4,EVFILT_READ,EV_ADD,0,0x0,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,{0x4,EVFILT_WRITE,EV_ADD,1,0x4,0x0},1,0x0,0,0x0) = 0 (0x0)
kevent(5,0x0,0,{0x4,EVFILT_WRITE,EV_EOF,0,0x2000,0x0},1,0x0) = 1 (0x1)
sendmsg(0x4,0x7fffd290,0x0,0x1,0x1,0x0) ERR#32 'Broken pipe'
SIGNAL 13 (SIGPIPE)
process exit, rval = 0
Apparently there is a bug that causes nscd to close the connection
immediately but even then it is wrong that this terminates the calling
program with SIGPIPE.
The below patch prevents the SIGPIPE but cannot revive the connection to
nscd. This may cause numeric UIDs in top or increase the load on the
directory server. It is compile tested only.
Index: lib/libc/net/nscachedcli.c
===
--- lib/libc/net/nscachedcli.c (revision 247710)
+++ lib/libc/net/nscachedcli.c (working copy)
@@ -75,9 +75,9 @@
nevents = _kevent(connection-write_queue, NULL, 0, eventlist,
1, timeout);
if ((nevents == 1) (eventlist.filter == EVFILT_WRITE)) {
- s_result = _write(connection-sockfd, data + result,
+ s_result = send(connection-sockfd, data + result,
eventlist.data data_size - result ?
- eventlist.data : data_size - result);
+ eventlist.data : data_size - result, MSG_NOSIGNAL);
if (s_result == -1)
return (-1);
else
@@ -175,8 +175,8 @@
nevents = _kevent(connection-write_queue, NULL, 0, eventlist, 1,
NULL);
if (nevents == 1 eventlist.filter == EVFILT_WRITE) {
- result = (_sendmsg(connection-sockfd, cred_hdr, 0) == -1) ?
- -1 : 0;
+ result = (_sendmsg(connection-sockfd, cred_hdr,
+ MSG_NOSIGNAL) == -1) ? -1 : 0;
EV_SET(eventlist, connection-sockfd, EVFILT_WRITE, EV_ADD,
0, 0, NULL);
_kevent(connection-write_queue, eventlist, 1, NULL, 0, NULL);
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] open_memstream() and open_wmemstream()
On Wed, Feb 13, 2013 at 11:44:19AM -0500, John Baldwin wrote:
On Thursday, February 07, 2013 4:12:22 pm Jilles Tjoelker wrote:
On Tue, Feb 05, 2013 at 03:46:43PM -0500, John Baldwin wrote:
I've written an implementation of open_memstream() and
open_wmemstream() along with a set of regression tests. I'm pretty
sure open_memstream() is correct, and I believe open_wmemstream() is
correct for expected usage. The latter might even do the right thing
if you split a multi-byte character across multiple writes. One
question I have is if my choice to discard any pending multi-byte
state in the stream anytime a seek changes the effective position in
the output stream. I think this is correct as stdio will flush any
pending data before doing a seek, so if there is a partially parsed
character we aren't going to get the rest of it.
I don't think partially parsed characters can happen with a correct
application. As per C99, an application must not call byte output
functions on a wide-oriented stream, and vice versa.
Oh, interesting. Should I remove those tests for using byte output
functions from the open_wmemstream test?
Yes. Unless real applications are identified that use this undefined
behaviour, such a test would test the implementation and not the
specification.
I briefly tried to run the tests against the glibc implementation
(Ubuntu 10.04) and got many failures. Glibc does not check the
parameters for null pointers and simply stores them blindly which is
POSIX-compliant. Things like bad length 6 for foo appear to be glibc
bugs, and test-open_wmemstream even segfaults.
Discarding the shift state on fseek()/fseeko() is permitted (but should
be documented as this is implementation-defined behaviour).
State-dependent encodings (where this is relevant) are rarely used
nowadays.
The conversion to bytes and back probably makes open_wmemstream() quite
slow but I don't think that is very important.
Note that we do this already for swprintf(). I've added this note:
IMPLEMENTATION NOTES
Internally all I/O streams are effectively byte-oriented, so using wide-
oriented operations to write to a stream opened via open_wmemstream()
results in wide characters being expanded to a stream of multibyte char-
acters in stdio's internal buffers. These multibyte characters are then
converted back to wide characters when written into the stream. As a
result, the wide-oriented streams maintain an internal multibyte charac-
ter conversion state that is cleared on any seek opertion that changes
the current position. This should have no effect unless byte-oriented
output operations are used on a wide-oriented stream.
This appears to approve the use of byte-oriented operations on a
wide-oriented stream which is in fact undefined.
http://www.FreeBSD.org/~jhb/patches/open_memstream.patch
The seek functions should check for overflow in the addition (for
SEEK_CUR and SEEK_END) and the conversion to size_t.
Note that SEEK_CUR is effectively only called with an offset of 0
via __ftello(). I'm not sure of the best way to check for overflow, do I
have
to do something like this:
static int
will_overflow(size_t off, fpos_t pos)
{
if (pos 0)
return (-pos off);
return (SIZE_MAX - off pos);
}
For SEEK_CUR I would test 'will_overflow(ms-offset, pos)' and
for SEEK_END 'will_overflow(ms-len, pos)'
Presumably SEEK_SET should test for the requested position being
beyond SIZE_MAX as well?
If my above test is ok then I end up with this:
Hmm, perhaps it is better to restrict to SSIZE_MAX instead of SIZE_MAX
as this greatly simplifies the code. With the above code, comparisons
may be signed or unsigned and there may be no greater type that fits
both a size_t and an fpos_t. Negating a negative fpos_t is also
incorrect because it may overflow. If you restrict the memory buffer to
ssize_t, an ssize_t can be safely converted to an fpos_t.
static int
will_overflow(size_t offset, fpos_t pos)
{
if (pos 0) {
if (-pos off) {
errno = EINVAL;
return (1);
}
} else {
if (SIZE_MAX - off pos) {
errno = EOVERFLOW;
return (1);
}
}
return (0);
}
static fpos_t
memstream_seek(void *cookie, fpos_t pos, int whence)
{
struct memstream *ms;
#ifdef DEBUG
size_t old;
#endif
ms = cookie;
#ifdef DEBUG
old = ms-offset;
#endif
switch (whence) {
case SEEK_SET:
if (pos SIZE_MAX) {
errno = EOVERFLOW;
return (-1);
}
ms-offset = pos;
break;
case SEEK_CUR:
if (will_overflow(ms-offset, pos))
return (-1);
ms
Re: [PATCH] open_memstream() and open_wmemstream()
On Tue, Feb 05, 2013 at 03:46:43PM -0500, John Baldwin wrote: I've written an implementation of open_memstream() and open_wmemstream() along with a set of regression tests. I'm pretty sure open_memstream() is correct, and I believe open_wmemstream() is correct for expected usage. The latter might even do the right thing if you split a multi-byte character across multiple writes. One question I have is if my choice to discard any pending multi-byte state in the stream anytime a seek changes the effective position in the output stream. I think this is correct as stdio will flush any pending data before doing a seek, so if there is a partially parsed character we aren't going to get the rest of it. I don't think partially parsed characters can happen with a correct application. As per C99, an application must not call byte output functions on a wide-oriented stream, and vice versa. Discarding the shift state on fseek()/fseeko() is permitted (but should be documented as this is implementation-defined behaviour). State-dependent encodings (where this is relevant) are rarely used nowadays. The conversion to bytes and back probably makes open_wmemstream() quite slow but I don't think that is very important. http://www.FreeBSD.org/~jhb/patches/open_memstream.patch The seek functions should check for overflow in the addition (for SEEK_CUR and SEEK_END) and the conversion to size_t. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: ktrace -d broken on current/stable-9
On Mon, Jan 14, 2013 at 06:48:13PM -0800, Garrett Cooper wrote: I tried using ktrace on a kernel compiled a week ago, and it appears to not be following forks like it should on amd64: # ktrace -d ./regress -l [snip] Not sure how it broke, but it was working a couple months ago (in particular I remember it working either around October or November), and the bug seems to have worked its way back to 9-STABLE (I'm running into the same problem if I do ktrace -d, enter a shell, then exec another shell from that shell). Haven't spent the time to bisect the commits looking for the culprit (yet), but if need be I'll trace down the culprit sometime this week. truss works, so it doesn't seem like ptrace(2) is broken. ktrace -d is not really useful in the synopsis with a command. It only means that the child processes of ktrace (at a time just before it executes the utility) should be traced as well. This is almost always an empty set, unless you do things like cmd1 ktrace -d cmd2 which will trace cmd2 and part of cmd1. You probably want ktrace -i. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: API explosion (Re: [RFC/RFT] calloutng)
On Wed, Dec 19, 2012 at 11:00:06AM +, Poul-Henning Kamp wrote: In message 50d192e8.3020...@freebsd.org, Alexander Motin writes: Linux uses 32.32 format in their eventtimers code. (And that is no accident, I know who they got the number from :-) But if at some point we want to be able to handle absolute wall time, [...] Then you have other problems, including but not limited to clock being stepped, leap-seconds, suspend/resume and frequency stability. If you want to support callouts of the type (At 14:00 UTC tomorrow) (disregarding the time-zone issue), you need to catch all significant changes to our UTC estimate and recalibrate your callout based on that. It is not obvious that we have applications for such an API that warrant the complexity. Either way, such a facility should be layered on top of the callout facility, which should always run in elapsed time[1] with no attention paid to what NTPD might do to the UTC estimate. POSIX specifies functions that assume such a facility exists, although applications may not care much if we implement them incorrectly. For example, pthread_mutex_timedlock() and sem_timedwait() shall time out when the CLOCK_REALTIME clock reaches the given value, and pthread_cond_timedwait() and clock_nanosleep() (with TIMER_ABSTIME) shall time out when the specified clock reaches the given value. So summary: 32.32 is the right format. Poul-Henning [1] Notice that elapsed time needs a firm definition with respect to suspend/resume, and that this decision has big implications for the API use and code duplication. I think it prudent to specify a flag to callouts, to tell what should happen on suspend/resume, something like: SR_CANCEL /* Cancel the callout on S/R */ /* no flag* /* Toll this callout only when system is running */ SR_IGNORE /* Toll suspended time from callout */ If you get this right, callouts from device drivers will just DTRT, if you get it wrong, all device drivers will need boilerplate code to handle S/R Userland could get access to this via CLOCK_REALTIME vs CLOCK_MONOTONIC vs CLOCK_UPTIME. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: ipv6_addrs_IF aliases in rc.conf(5)
On Thu, Dec 20, 2012 at 01:04:34PM +0200, Kimmo Paasiala wrote: A question related to this for those who have been doing work on the rc(8) scripts. Can I assume that /usr/bin is available when network.subr functions are used? Doing calculations on hexadecimal numbers is going to be very awkward if I can't use for example bc(1). You cannot assume that /usr/bin is available when setting up the network. It may be that /usr is mounted via NFS. You can use hexadecimal numbers (prefixed with 0x) in $((...)) expressions. In FreeBSD 9.0 or newer, sh has a printf builtin you can use; in older versions you can use hexdigit and hexprint from network.subr. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: rev 244030 route command is not working
On Tue, Dec 11, 2012 at 04:07:03PM +0400, Gleb Smirnoff wrote:
On Tue, Dec 11, 2012 at 12:21:20PM +0200, Artyom Mirgorodskiy wrote:
A Gleb, when I reset errno at the begin of fiboptlist_csv()
A everything work as expected.
Artyom,
can you please test attached patch?
Index: route.c
===
--- route.c (revision 244082)
+++ route.c (working copy)
@@ -271,8 +271,7 @@
case 0:
case 1:
fib[i] = strtol(token, endptr, 0);
- if (*endptr != '\0' || (fib[i] == 0
- (errno == EINVAL || errno == ERANGE)))
+ if (*endptr != '\0')
error = 1;
break;
default:
@@ -336,8 +335,7 @@
goto fiboptlist_csv_ret;
} else {
fib = strtol(token, endptr, 0);
- if (*endptr != '\0' || (fib == 0
- (errno == EINVAL || errno == ERANGE))) {
+ if (*endptr != '\0') {
error = 1;
goto fiboptlist_csv_ret;
}
This patch will avoid erroneously failing but will let through certain
invalid strings. The empty string will silently be treated as 0 and
values outside the range [LONG_MIN, LONG_MAX] will be clamped silently
(the latter was already broken in most cases because [ERANGE] happens
only with a return value of LONG_MIN or LONG_MAX).
Other invalid strings that were and are let through (with unexpected
results) are ones containing a number that fits in a long but not in an
int.
To fix the range detection, errno should be set to 0 before the call and
checked afterwards; in a library function (this is an application),
errno should be saved and restored around that to avoid setting errno to
0 as a side effect of the function. The empty string needs a specific
check.
I don't insist on this being fixed but it shows that strtol() is too
hard to use correctly. The non-standard strtonum() looks easier but has
other problems (such as returning an English string in an API that
should be language-neutral).
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: RFC: sysctl -f filename
,
+ string, line);
+ return (1);
default:
- warn(%s, string);
- warncount++;
- return;
+ warn(%s%s, string, line);
+ return (1);
}
}
if (!bflag)
@@ -332,8 +371,46 @@
putchar('\n');
nflag = i;
}
+
+ return (0);
}
+static int
+parsefile(const char *filename)
+{
+ FILE *file;
+ char line[BUFSIZ], *p;
+ int warncount = 0, lineno = 0;
+
+ file = fopen(filename, r);
+ if (file == NULL)
+ err(EX_NOINPUT, %s, filename);
+ while (fgets(line, sizeof(line), file) != NULL) {
+ lineno++;
+ p = line;
+ /* Replace the first # with \0. */
+ while((p = strchr(p, '#')) != NULL) {
sh(1) only recognizes # specially at the beginning of a word but perhaps
you want to do it differently.
+ if (p == line || *(p-1) != '\\') {
This '\\' thing looks a bit strange. There is no backslash removal later
on.
+ *p = '\0';
+ break;
+ }
+ p++;
+ }
+ p = line;
+ while (isspace((int)p[strlen(p) - 1]))
If strlen(p) == 0, then this is an access out of bounds.
The cast should be (unsigned char) not (int).
+ p[strlen(p) - 1] = '\0';
+ while (isspace((int)*p))
+ p++;
+ if (*p == '\0')
+ continue;
+ else
+ warncount += parse(p, lineno);
+ }
+ fclose(file);
+
+ return (warncount);
+}
+
/* These functions will dump out various interesting structures. */
static int
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: clang and static linking?
On Sat, Nov 10, 2012 at 01:33:40AM +0100, Dimitry Andric wrote:
On 2012-11-10 00:25, Dimitry Andric wrote:
...
The more difficult way out is to not define any duplicate functions in
libc.a and libm.a. For the shared libraries, this should not be a
problem, since the dynamic linker will figure out which of the two
copies will get precedence. The functions must stay available for
backwards compatibility reasons anyway.
For static libraries, this compatibility seems to be unnecessary, as
they will only be used to link new programs. Therefore, it would
probably be best to remove the whole isnan.o member from libc.a, and
move all the isnan functions to libm.a instead.
Currently, isnan() is commented out in lib/msun/src/s_isnan.c, maybe we
can enable it whenever PIC is not defined? Then we could simply skip
building lib/libc/gen/isnan.c for libc.a.
More concretely, here is a patch that seems to achieve the above:
I see this has already been committed, but anyway...
- Only define isnan, isnanf, __isnan and __isnanf in libc.so, not in
libc.a and libc_p.a.
OK, but please add a comment about this.
- Define isnan in libm.a and libm_p.a, not in libm.so. I don't think
there is a need to define __isnan in the .a files, so I left that out.
Removing symbols from a .so causes subtle ABI breakage and is not needed
for fixing static linking.
More concretely, dlsym of isnan on libm.so will stop working and a
different version of isnan will be chosen if the search list is libm.so,
libother.so, libc.so and libother.so contains another isnan.
Index: lib/libc/gen/isnan.c
===
--- lib/libc/gen/isnan.c (revision 242841)
+++ lib/libc/gen/isnan.c (working copy)
@@ -35,6 +35,7 @@
* binary compat until we can bump libm's major version number.
*/
+#ifdef PIC
__weak_reference(__isnan, isnan);
__weak_reference(__isnanf, isnanf);
@@ -55,3 +56,4 @@ __isnanf(float f)
u.f = f;
return (u.bits.exp == 255 u.bits.man != 0);
}
+#endif /* PIC */
Index: lib/msun/src/s_isnan.c
===
--- lib/msun/src/s_isnan.c(revision 242841)
+++ lib/msun/src/s_isnan.c(working copy)
@@ -30,8 +30,9 @@
#include fpmath.h
-/* Provided by libc */
-#if 0
+/* Provided by libc.so */
+#ifndef PIC
+#undef isnan
int
isnan(double d)
{
@@ -40,7 +41,7 @@ isnan(double d)
u.d = d;
return (u.bits.exp == 2047 (u.bits.manl != 0 || u.bits.manh != 0));
}
-#endif
+#endif /* !PIC */
int
__isnanf(float f)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: pkgng suggestion: renaming /usr/sbin/pkg to /usr/sbin/pkg-bootstrap
On Sat, Sep 01, 2012 at 08:40:03PM +0100, Matthew Seaman wrote: On 01/09/2012 18:43, Tijl Coosemans wrote: In this scenario the ports tree needs to keep support for older releases, but that's a consequence of the fact that there's only one ports tree for all releases. Somewhere in between the ports and the various releases there has to be some form encapsulation, not just for pkg, but for all the tools used by the ports tree. Given how the ports tree currently encapsulates both the old and new pkg tools I don't see how supporting multiple versions of pkgng would be a problem because presumably the difference between pkgng versions is going to be much smaller than the difference between the old and new tools. New functionality already in the process of development will entail making non-backwards compatible changes to the DB schema. If we're tied to supporting a version of pkgng bundled with a release, that's going to make rolling out such changes much harder. On the other hand, if pkgng is in ports, then we can release a new version and simultaneously publish new package sets (incorporating the update to pkgng) from the repositories which will have been built using the updated DB schema. One restriction is that the old pkg should be able to detect the new pkg package in the repository. If not, users of an old version will have to install the new pkg manually. This restriction is much weaker than requiring an upgrade using the old pkg, mainly because pkg does not depend on anything else. All other packages can use features and bugfixes in the new pkg and need not carry around workarounds for years. If the base system is also managed by pkg, which is currently not the case, the assumption that the pkg package does not depend on anything may no longer hold. It might depend on a new libc in the new base system, for example. The ports tree doesn't track the versioning of the base system, and it makes perfect sense to me that tools for dealing with the ports should follow changes to ports rather than changes to the base. Yes. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] Add a -h flag to mv
On Tue, Aug 28, 2012 at 10:58:09AM -0400, John Baldwin wrote: I have a use case at work where I need to be able to update a symlink that points to a directory atomically (so that it points to a new directory). To give a conrete example, suppose I have two directories 'foo' and 'bar', and a symlink 'a' that I wish to atomically flip from 'foo' to 'bar'. Using 'ln -shf bar a' is not atomic as it uses separate unlink() and symlink() system calls, so there is a race where another thread may encounter ENOENT while traversing 'a'. The approach we used was to create a new symbolic link 'a.new' (e.g. via 'ln -s bar a.new') and then use rename() to rename 'a.new' on top of 'a'. Normally to do an atomic rename from userland one would use 'mv', but 'mv a.new a' doesn't do that. Instead, it moves 'a.new' into the directory referenced by the 'a' symlink. At work we have resorted to invoking python's os.rename() in a one-liner to handle this. While rehashing this discussion today it occurred to me that a -h flag to mv would allow it to work in this case (and is very similar to how ln treats its -h flag). To that end, I have a patch to add a new -h flag to mv that allows one to atomically update a symlink that points to a directory. I could not find any other mv commands that have adopted a -h (or a different flag that accomplishes the same task). Given that it functions identically to the -h flag for ln, -h seemed the logical choice. Any objections? GNU coreutils mv (and also cp/install/ln) appears to use -T/--no-target-directory for a similar purpose: -T prevents the target being treated as a directory (whether it is a symlink or not). -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] Add a -h flag to mv
On Wed, Aug 29, 2012 at 08:09:20AM -0400, John Baldwin wrote: On Wednesday, August 29, 2012 6:02:47 am Jilles Tjoelker wrote: GNU coreutils mv (and also cp/install/ln) appears to use -T/--no-target-directory for a similar purpose: -T prevents the target being treated as a directory (whether it is a symlink or not). Hmm, I could find no documentation for this online via Google searches or the Linux manpages we have on www.FreeBSD.org. Bah, Google just makes searching for these sorts of things painful it seems (you have to put explicit quotes around --no-target-directory for it to actually be used). Also, it seems I just chose all the wrong Linux manpage sets to look at. It seems that Linux's -T flag is similar to -h for ln as well. I don't think we can deprecate -h for ln, but perhaps we could add -T as a compat flag to ln and mv? I'd be inclined to still add -h to mv so that it mirrors ln. Hmm, it seems RedHat's ln uses -n for this (OpenBSD, NetBSD, and Darwin all include a -n as an alias to -h for ln to support compat with other operating systems). OSF/1 (and Tru64) and SunOS use -n to mean complain if the file already exists similar to 'mv -n'. Also, looking at the Suse manpage on www.FreeBSD.org, it seems their ln (which does have -T) has both -n and -T with different descriptions, but to achieve the same purpose: http://www.freebsd.org/cgi/man.cgi?query=lnapropos=0sektion=0manpath=SuSE+Linux%2Fi386+11.3arch=defaultformat=html -n, --no-dereference treat destination that is a symlink to a directory as if it were a normal file -T, --no-target-directory treat LINK_NAME as a normal file (To me it seems LINK_NAME and destination are the same thing.) My inclination would be to add -h to mv, but perhaps add -T as an alias for -h to both ln and mv, and -n as an alias for -h to ln (if we want aliases to match coreutils). Coreutils ln -n is the same as our ln -h, and we already have compatibility for it. The coreutils -T option is different, though. It forces the ln source_file target_file synopsis instead of the ln source_file ... target_dir synopsis, without checking the type of the final operand. If there are not exactly two operands, a syntax error occurs. If the final operand is a directory and cannot be overwritten, an error occurs. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: pkgng suggestion: renaming /usr/sbin/pkg to /usr/sbin/pkg-bootstrap
On Sat, Aug 25, 2012 at 06:34:43PM -0500, CyberLeo Kitsana wrote: On 08/24/2012 07:01 PM, Baptiste Daroussin wrote: Can anyone give me he details on the security related problem? Off the top of my head, it seems to represent a break in the chain of trust: how does the bootstrapper verify that the tarball it just downloaded to bootstrap pkg is genuine, and not, for example, a trojan? The source in usr.sbin/pkg/pkg.c[1] doesn't seem to suggest it cares. Indeed it does not care, and the current security features are insufficient (unless the bootstrapper can use the signed sqlite db to verify the pkg package). I think the fix is to modify 'pkg repo' so it detects the pkg package and creates a separate signature for it which can be verified by the bootstrapper, without needing sqlite. The public key for this signature will have to be distributed with base (like the public keys for freebsd-update and portsnap). -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: pkgng default schedule... registering a few reasons for rethinking the final implementation...
On Thu, Aug 23, 2012 at 03:50:11PM -0400, Kris Moore wrote:
Well, it was about time I got to doing a benchmark of this anyway :)
I did quick benchmark of how one of our utilities parses through a list
of 1k packages on a newer i5 system:
First test, using /var/db/pkg/pkg check we have been doing:
0.178s 0:00.31 54.8%
0.123s 0:00.26 61.5%
0.099s 0:00.15 60.0%
Second test, using pkg info pkg:
5.347s 0:11.41 91.7%
5.444s 0:11.52 91.3%
5.878s 0:11.32 91.4%
The pkg info command is quite a bit slower in this case, but 5 seconds
isn't horrible.
[snip]
The only way around It I've found is to do a quick pkg info on the
entire DB, dump that to a list, then begin to grep through that list for
each item, but it still takes 10~ seconds on the atom. That may be what
I end up having to do, but it still stinks to go from a half a second
startup, to 10 seconds each time. Any other ideas on how to do this
faster with the new pkgng?
Don't use grep: the list is not big enough to make it worth it.
What should work fairly efficiently is to store a list of packages in a
shell variable once and then check each sub-package without external
programs.
list=$(pkg query %n-%v)
for pkgwithversion in ...; do
case $'\n'$list$'\n' in
*$'\n'$pkgwithversion$'\n'*) echo yes ;;
*) echo no ;;
esac
done
This does assume that the list does not change during the loop.
Also, instead of
pName=`echo $pkg | rev | cut -d - -f 2-25 | rev`
try
pName=${pkg%-*}
and use arithmetic expansion ($((...))) instead of invoking expr where
possible.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: PAM passwdqc, strict aliasing, and WARNS
On Fri, Jul 13, 2012 at 04:14:17PM -0600, Justin T. Gibbs wrote: Someone who has yet to confess added -Werror to the global CFLAGS (via /etc/make.conf) for one of our systems at work. Before I figured out that this was the cause of builds failing, I hacked up pam_passwdc to resolve the problem. This gets the module to WARNS=2, but to go farther, the logically const issues with this code will need to be sorted out. Is this change worth committing? Is this the best way to resolve the strict aliasing issues in this code? The prototype of pam_get_item() is int pam_get_item(const pam_handle_t *pamh, int item_type, const void **item); Therefore, you should pass a pointer to a const void pointer as the last argument. You can then convert the const void pointer to the desired type. For example: const void *item; const struct pam_conv *conv; result = pam_get_item(pamh, PAM_CONV, item); conv = item; Passing something like a pointer to a 'const struct pam_conv *' to pam_get_item() will cause a strict-aliasing violation because pam_get_item() will attempt to store a value into an object of declared type 'const struct pam_conv *' using an lvalue of type 'const void *'. In both C99 and C11, these rules are in 6.5 Expressions. In the case of const struct pam_conv *, the union approach violates the C standard because the C standard does not guarantee that all object pointers have the same representation. The conversion might be non-trivial and a type-pun may not work properly. However, in almost all real machines the conversion is trivial. Some compilers may still consider the union approach a strict-aliasing violation. In any case, I think it is a bit ugly and should be avoided when possible (like here). -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: make delete-old performance.
On Thu, May 17, 2012 at 02:13:40PM +0200, Dimitry Andric wrote:
On 2012-05-17 05:18, b. f. wrote:...
The slowdown is probably due - at least in part - to two factors:
- the list of files to be checked for removal has grown substantially,
because missing entries for old knobs and new entries for new knobs
have been added; and
- a new (and slower) method of checking was added in:
http://svnweb.FreeBSD.org/base?view=revisionrevision=220255
because the old method broke down with the size of the new lists of files.
Hm, maybe it would have been better to fix make, so it can accept
arbitrarily long lists, without segfaulting? There's such a thing as
malloc(), and I simply don't believe any of those lists could be larger
than a few hundred kilobytes.
Alternatively, make could be fixed so that the original code works.
Although an invocation like
sh -c 'for file in VERY_LONG_LIST; do something; done'
will bump into {ARG_MAX}, the shell itself does not have a fixed
limitation so longer command lines can be written to a temporary file
and passed to sh that way.
In some cases (such as with -j), make always uses a temporary file,
slowing things down and obscuring ps output.
At the cost of needing the temporary file named a bit longer, it is
better to pass the pathname to sh rather than feeding the script on
standard input: this avoids interfering with terminal input and is
potentially faster.
The code currently in Makefile.inc1 can probably be sped up by passing
the output of the make -V command to something like
xargs sh -c 'for file do rm -i ${DESTDIR}/${file}; done' sh
instead of the xargs -n1 | while read file; do ...; done loop.
(Note the second sh at the end, which serves as a value for $0 so all
strings from xargs become positional parameters.)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Build error in bin/sh/jobs.c if DEBUG=2
On Sun, Apr 01, 2012 at 04:14:24PM +0200, Kristof Provost wrote: While chasing down an odd issue with alignment faults I activated debugging in bin/sh. bin/sh/Makefile has a commented out line (# DEBUG_FLAGS+= -g -DDEBUG=2 -fno-inline) to do this so that's what I did. This fails to compile in bin/sh/jobs.c in vforkexecshell(). The debug TRACE() tries to print variables which don't exist. The patch below fixes the compilation problem, but I'm unsure if it's printing the relevant information. Thanks, I committed a fix. I fairly arbitrarily chose some information to print, since I do not use -DDEBUG=2 myself. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Enhancing the user experience with tcsh
On Sun, Feb 12, 2012 at 04:05:14AM -0300, Gonzalo Nemmi wrote: Joel, with all due respect, do you really think that 99.9% of all users will not find the _non_intrusive_ additions below useful? bindkey \e[1~ beginning-of-line #make Home key work; bindkey \e[2~ overwrite-mode #make Ins key work; bindkey \e[3~ delete-char #make Delete key work; bindkey \e[4~ end-of-line #make End key work; ... I mean, after all, setting those keys do not change the behaviour nor the output of any given command, they are present in 99.9% of the keyboards we all get to see everyday and they do not work under the current .cshrc config. Im not talking about an improvement, making things easier for new users or experience improvement of any kind ... Im talking about including them so all users get to have a fully functional keyboard by default. I think this kind of basic stuff should work without any configuration; it should be fixed in the tcsh code if it does not work already. It looks like Home and End already work in the common configurations (xterm and cons25), so bindkey is unnecessary for them. Delete should be fixed in tcsh like I fixed it in libedit in r212235, which will make it work in xterm but not cons25. If the 7.x/8.x syscons is important enough, further tweaking may be appropriate. The Ins key is more questionable because I think it is not used deliberately by many people but is annoying if you accidentally press it and do not realize. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: stray symbol in hd's output?
On Sun, Jan 29, 2012 at 11:07:41PM +0200, Andriy Gapon wrote: Does the following look familiar to anybody (note the stray 'n')? $ dd if=/dev/zero bs=1 count=1 | hd 1+0 records in 1+0 records out 1 bytes transferred in 0.43 secs (23302 bytes/sec) 00|.| n0001 Yes, this was broken by r229794 (Jan 7) and repaired again by r230649 (yesterday). http://www.freebsd.org/cgi/query-pr.cgi?pr=144722 -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: i18n and shell scripts
On Sun, Jan 22, 2012 at 04:05:10PM -0600, Ron McDowell wrote:
I'm working on the new bsdconfig, and looking for some good examples of
how to incorporate internationalization into the scripts. I'm not
finding much love in this area. :-(
Any pointers appreciated.
Yes, this is not very easy, and particularly not if you only want to use
base system functionality.
An important thing is printf(1) as it will let you put placeholders for
variables in translatable strings rather than build strings from hard to
understand pieces. (Our printf(1) does not support changing the order of
variables like the printf(3) function does.)
With GNU gettext (which is in ports, not in base), you can translate
messages in a shell script much like in a C program, although you will
be invoking the gettext(1) tool rather frequently, which may be slow.
It has many features to help the programmer and the translator. This
also means that it can be fairly complicated to use.
Alternatively, you could do it yourself. One way would be to define a
variable for each translatable string with the default language's value,
then source a file for the appropriate language
(mylang=${LC_ALL-${LC_MESSAGES-${LANG}}}). Supporting multiple character
sets is going to be ugly because we do not have any form of iconv
(character set conversion) in base. This is particularly bad because
syscons does not support UTF-8 while many users want to use UTF-8 in X
instead of language-specific character sets.
The base catgets(3) facility does not provide a utility for use from
shell scripts, and is also harder to use than gettext.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: HEADS UP: set_rcvar() removed from rc.subr
On Sat, Jan 14, 2012 at 01:05:59AM -0800, Doug Barton wrote: Per discussion in freebsd-rc@, I have removed set_rcvar() from rc.subr. The concept of set_rcvar() was nice in theory, but the forks it creates are a drag on the startup process, which is especially noticeable on slower systems, such as embedded ones. I have no plans to MFC this change, so it should only affect users who are actually on 10-current. If you have scripts in /usr/local/etc/rc.d (which if you have ports installed you almost certainly do) ... to make the change by hand, change this: name=foo rcvar=`set_rcvar` to: name=foo rcvar=foo_enable I didn't bump PORTREVISIONs because the change only applies to HEAD. But all of the ports are updated, so if you can't figure out how to make the change, just reinstall it. Why must the 2-argument form of set_rcvar die at the same time? It is used very differently and does not cause unnecessary forks. Instead, it is called in the same shell environment to define additional rc.conf variables that have defaults and are shown in 'script rcvar'. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: add 'ldd' to cross-tools ?
On Wed, Jan 04, 2012 at 11:58:15PM +0100, Luigi Rizzo wrote:
On Wed, Jan 04, 2012 at 02:30:27PM -0800, Garrett Cooper wrote:
On Wed, Jan 4, 2012 at 2:29 PM, Luigi Rizzo ri...@iet.unipi.it wrote:
...
$ objdump -x `which tar` | awk '$1 == NEEDED { print $2 }'
libarchive.so.5
libbz2.so.4
libz.so.6
liblzma.so.5
libbsdxml.so.4
libcrypto.so.6
libc.so.7
wonderful, thanks!
Np! The only gap with both of these tools is that you have to
watch out for dl_open'ed binaries as they won't show up in ldd/objdump
-x. If I could figure out how to detect these with a command line
tool, I would be set for life :).
and the other thing, i just realized, is that once you locate
the libraries you should run objdump recursively to find
out further dependencies. Perhaps ldd sorts this out by itself ?
ldd basically sets LD_TRACE_LOADED_OBJECTS=yes and runs the program
(after having checked it is a dynamic executable). This means it will
not work as a cross tool. Upsides are that it is simple and it shows
exactly what rtld would do (because it is rtld), handling things like
/var/run/ld-elf.so.hints, LD_LIBRARY_PATH and pathnames hardcoded into
objects.
You will have to run objdump (or readelf) recursively. (Note that there
are also use cases where just the non-recursive NEEDED tags are
appropriate, not all objects that happen to be loaded.)
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: r228700 can't dhclient em0
On Tue, Dec 20, 2011 at 11:23:54PM +0400, Gleb Smirnoff wrote: Considering r228571: we need to specify vhid as additional address attribute in atomic manner, via one ioctl(). Address can't be first configured, and then made redundant, that would lead it to being static for a short period, sending gratutious ARP announcement, etc. An assumption that we are not allowed to change ABI for our own tools strongly discourages bringing in new features :( Consider changing to a more flexible ABI that does not need to be broken for new features. Examples are nmount(2)'s name-value pairs and GEOM's XML topology descriptions. This is initially more work and has poorer performance, but it may well be worth it. Process information reserves space in structures for future extension; this is less flexible but performs better (which matters somewhat). Another consideration is compatibility for 32-bit applications on 64-bit kernels; a good ABI design will minimize the amount of code needed to support that. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: SCHED_ULE should not be the default
On Tue, Dec 13, 2011 at 10:40:48AM +0200, Ivan Klymenko wrote:
If the algorithm ULE does not contain problems - it means the problem
has Core2Duo, or in a piece of code that uses the ULE scheduler.
I already wrote in a mailing list that specifically in my case (Core2Duo)
partially helps the following patch:
--- sched_ule.c.orig 2011-11-24 18:11:48.0 +0200
+++ sched_ule.c 2011-12-10 22:47:08.0 +0200
@@ -794,7 +794,8 @@
* 1.5 * balance_interval.
*/
balance_ticks = max(balance_interval / 2, 1);
- balance_ticks += random() % balance_interval;
+// balance_ticks += random() % balance_interval;
+ balance_ticks += ((int)random()) % balance_interval;
if (smp_started == 0 || rebalance == 0)
return;
tdq = TDQ_SELF();
This avoids a 64-bit division on 64-bit platforms but seems to have no
effect otherwise. Because this function is not called very often, the
change seems unlikely to help.
@@ -2118,13 +2119,21 @@
struct td_sched *ts;
THREAD_LOCK_ASSERT(td, MA_OWNED);
+ if (td-td_pri_class PRI_FIFO_BIT)
+ return;
+ ts = td-td_sched;
+ /*
+ * We used up one time slice.
+ */
+ if (--ts-ts_slice 0)
+ return;
This skips most of the periodic functionality (long term load balancer,
saving switch count (?), insert index (?), interactivity score update
for long running thread) if the thread is not going to be rescheduled
right now.
It looks wrong but it is a data point if it helps your workload.
tdq = TDQ_SELF();
#ifdef SMP
/*
* We run the long term load balancer infrequently on the first cpu.
*/
- if (balance_tdq == tdq) {
- if (balance_ticks --balance_ticks == 0)
+ if (balance_ticks --balance_ticks == 0) {
+ if (balance_tdq == tdq)
sched_balance();
}
#endif
The main effect of this appears to be to disable the long term load
balancer completely after some time. At some point, a CPU other than the
first CPU (which uses balance_tdq) will set balance_ticks = 0, and
sched_balance() will never be called again.
It also introduces a hypothetical race condition because the access to
balance_ticks is no longer restricted to one CPU under a spinlock.
If the long term load balancer may be causing trouble, try setting
kern.sched.balance_interval to a higher value with unpatched code.
@@ -2144,9 +2153,6 @@
if (TAILQ_EMPTY(tdq-tdq_timeshare.rq_queues[tdq-tdq_ridx]))
tdq-tdq_ridx = tdq-tdq_idx;
}
- ts = td-td_sched;
- if (td-td_pri_class PRI_FIFO_BIT)
- return;
if (PRI_BASE(td-td_pri_class) == PRI_TIMESHARE) {
/*
* We used a tick; charge it to the thread so
@@ -2157,11 +2163,6 @@
sched_priority(td);
}
/*
- * We used up one time slice.
- */
- if (--ts-ts_slice 0)
- return;
- /*
* We're out of time, force a requeue at userret().
*/
ts-ts_slice = sched_slice;
and refusal to use options FULL_PREEMPTION
But no one has unsubscribed to my letter, my patch helps or not in the
case of Core2Duo...
There is a suspicion that the problems stem from the sections of code
associated with the SMP...
Maybe I'm in something wrong, but I want to help in solving this
problem ...
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: WITHOUT_PROFILE=yes by default
On Mon, Nov 28, 2011 at 05:38:20PM +0700, Max Khon wrote: I would like to disable building profiled libraries by default. Opinions? Agreed. There are better profiling tools available now that do not require recompiling the program with special options and statically linking it. Examples are pmcstat and callgrind/cachegrind. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: possible array out of bounds access in sys/netinet/sctp_output.c
On Sun, Nov 27, 2011 at 03:45:36PM +, Alexander Best wrote: i've been playing with clang tot and noticed the following error: /usr/local/bin/clang -c -O3 -pipe -fno-inline-functions -fno-strict-aliasing -march=core2 -std=c99 -g -fdiagnostics-show-option -fformat-extensions -Wall -Wcast-qual -Winline -Wmissing-include-dirs -Wmissing-prototypes -Wnested-externs -Wpointer-arith -Wredundant-decls -Wstrict-prototypes -Wundef -Wno-pointer-sign -nostdinc -I. -I/usr/git-freebsd-head/sys -I/usr/git-freebsd-head/sys/contrib/altq -D_KERNEL -DHAVE_KERNEL_OPTION_HEADERS -include opt_global.h -fno-omit-frame-pointer -mno-aes -mno-avx -mcmodel=kernel -mno-red-zone -mno-mmx -msoft-float -fno-asynchronous-unwind-tables -ffreestanding -Wno-error=tautological-compare -Wno-error=shift-count-negative -Wno-error=shift-count-overflow -Wno-error=shift-overflow -Wno-error=conversion -Wno-error=empty-body -Wno-error=gnu-designator -Wno-error=format -Wno-error=format-invalid-specifier -Wno-error=format-extra-args -Werror /usr/git-freebsd-head/sys/netinet/sctp_output.c clang: warning: argument unused during compilation: '-fformat-extensions' /usr/git-freebsd-head/sys/netinet/sctp_output.c:4685:2: error: array index 1 is past the end of the array (which contains 1 element) [-Werror,-Warray-bounds] sup_addr-addr_type[1] = htons(SCTP_IPV6_ADDRESS); ^ ~ /usr/git-freebsd-head/sys/netinet/sctp_header.h:84:2: note: array 'addr_type' declared here uint16_t addr_type[SCTP_ARRAY_MIN_LEN]; /* array of supported address ^ 1 error generated. *** Error code 1 Stop in /usr/obj/usr/git-freebsd-head/sys/GENERIC. *** Error code 1 Stop in /usr/git-freebsd-head. *** Error code 1 Stop in /usr/git-freebsd-head. this is from a GENERIC kernel build (so INET + INET6) for amd64. is this a false positive, or is length(sup_addr-addr_type) really == 1, thus making sup_addr-addr_type[1] an illegal access? This is the fairly common construct of a variable-length array at the end of a struct. With C89, this was not allowed but defining one element and allocating more elements worked in most implementations. C99 recognized this need and created a way to do it, which looks like uint16_t addr_type[];. This adds any necessary padding and allows access to however many elements have been allocated. Also, if it is not at the end of a struct it is an error. Using this new construct requires code changes because some code such as fairly close to the error message relies on the size of the one element already in the struct. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: crash in sb-concurrency tests after r216641 on x86-64/freebsd9/sb-thread
On Wed, Oct 12, 2011 at 12:00:07AM +, Nali Toja wrote:
After r216641 sbcl built with sb-thread dies on mailbox tests. It also
dies when I try to complete a symbol in slime. The workaround seems to
be to revert libthr to r216640.
http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/154050
http://svn.freebsd.org/changeset/base/216641
http://www.freshports.org/lang/sbcl # or see ports/161444 for sbcl-1.0.52
Any clue whether it's a FreeBSD bug or a SBCL bug? I've Bcc'd sbcl-bugs@
in case it's the latter one.
[snip]
Fatal error 'thread was already on queue.' at line 222 in file
/usr/src/lib/libthr/thread/thr_cond.c (errno = 0)
[snip]
(gdb) bt
#0 0x000800c5c7ec in _umtx_op_err () at
/usr/src/lib/libthr/arch/amd64/amd64/_umtx_op_err.S:37
#1 0x000800c5423e in _thr_umtx_timedwait_uint (mtx=0x8006d4ea8, id=0,
clockid=0, abstime=0x0, shared=0) at /usr/src/lib/libthr/thread/thr_umtx.c:214
#2 0x000800c5c04b in _thr_sleep (curthread=0x828d5d400, clockid=0,
abstime=0x0) at /usr/src/lib/libthr/thread/thr_kern.c:212
#3 0x000800c5f5dd in cond_wait_user (cvp=0x828fdf850, mp=0x828fe0970,
abstime=0x0, cancel=1) at /usr/src/lib/libthr/thread/thr_cond.c:243
#4 0x000800c5f856 in cond_wait_common (cond=0x8480f0008,
mutex=0x8480f, abstime=0x0, cancel=1) at
/usr/src/lib/libthr/thread/thr_cond.c:299
#5 0x000800c5f8b7 in __pthread_cond_wait (cond=0x8480f0008,
mutex=0x8480f) at /usr/src/lib/libthr/thread/thr_cond.c:313
#6 0x0008009e9fa0 in pthread_cond_wait_exp (p0=0x8480f0008,
p1=0x8480f) at /usr/src/lib/libc/gen/_pthread_stubs.c:217
#7 0x00413574 in wait_for_thread_state_change (thread=0x8480f0010,
state=16) at thread.h:53
#8 0x004133a8 in sig_stop_for_gc_handler (signal=31,
info=0x847eef630, context=0x847eef2c0) at interrupt.c:1265
#9 0x0041427d in low_level_handle_now_handler (signal=31,
info=0x847eef630, void_context=0x847eef2c0) at interrupt.c:1729
#10 0x7023 in ?? ()
#11 0x00414220 in low_level_unblock_me_trampoline () at
interrupt.c:1723
#12 0x00100154c990 in ?? ()
#13 0x0063eaa0 in interrupt_handlers ()
#14 0x000200411d4f in ?? ()
#15 0x001003375721 in ?? ()
#16 0x38b48504001a in ?? ()
#17 0x000a81f0 in ?? ()
#18 0x in ?? ()
#19 0x000847eef840 in ?? ()
#20 0x001003af2a2f in ?? ()
#21 0x002004e9c3e1 in ?? ()
#22 0x000800c58570 in _sigprocmask (how=Could not find the frame base
for _sigprocmask.
) at /usr/src/lib/libthr/thread/thr_sig.c:584
Previous frame inner to this frame (corrupt stack?)
(gdb) f 7
#7 0x00413574 in wait_for_thread_state_change (thread=0x8480f0010,
state=16) at thread.h:53
53 pthread_cond_wait(thread-state_cond, thread-state_lock);
(gdb) l
48 static inline void
49 wait_for_thread_state_change(struct thread *thread, lispobj state)
50 {
51 pthread_mutex_lock(thread-state_lock);
52 while (thread-state == state)
53 pthread_cond_wait(thread-state_cond, thread-state_lock);
54 pthread_mutex_unlock(thread-state_lock);
55 }
56
57 extern pthread_key_t lisp_thread;
The cause of the trouble appears to be that pthread_cond_wait() is
interrupted by a signal handler and the signal handler calls
pthread_cond_wait() again (no matter whether it is on the same or a
different condition variable). POSIX forbids this because (like most of
the pthread functions) pthread_cond_wait() is not async-signal-safe.
While the pre-r216641 code is not async-signal-safe either, it would
usually work fine. With the r216641 code, the second call to
pthread_cond_wait() aborts immediately with the 'thread was already on
queue' message.
The immediate issue could be fixed in libthr fairly easily by enabling
its code to postpone signal handlers also during pthread_cond_wait() (a
signal will still interrupt the wait). However, this does not fix issues
due to signal handlers interrupting other pthread functions which may
still cause erratic undefined behaviour. Therefore, it may not be
desirable to do this.
An alternative is to use pthread_suspend_np(). This function will wait
for the thread to stop before returning, although it may stop almost
anywhere. I have not tried this but calling it on a thread in
pthread_cond_wait() should be safe.
Ideally, it would not be necessary to stop all other threads while
collecting garbage, but this may be hard to fix.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [Freebsd 9] [amd64] [USB] [HPLIP] what's the (new) right way to manage hplip usb-plugged printers, running Freebsd 9
On Sat, Oct 29, 2011 at 04:10:46PM +0200, David Marec wrote: So, what's should be the news groupuser's rights required by HPLIP/cups on FreeBSD 9 ? And, how to handle them with devd ? Use devfs rules. Pasting from http://www.stack.nl/~jilles/unix/freebsd-devfs.txt Create or edit /etc/devfs.rules and put something like this in it: [devfsrules_mybox=10] add path 'fd0*' mode 660 See man 8 devfs for more information. Then put in /etc/rc.conf devfs_system_ruleset=devfsrules_mybox If you want to edit other /dev mountpoints (e.g. for jails) use something like devfs_set_rulesets=/usr/jails/jail1/dev=devfsrules_jail1 /usr/jails/jail2/dev=devfsrules_jail2 -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Not setting TERM explicitly wraps commands at 80 columns with nested shells in xterms using sh + bash?
On Fri, Oct 21, 2011 at 12:52:17AM -0700, Garrett Cooper wrote: # Increased the window size here. [gcooper@fallout ~]$ uname -a FreeBSD fallout.local 10.0-CURRENT FreeBSD 10.0-CURRENT #1 r226332M: Wed Oct 12 22:48:55 PDT 2011 root@fallout.local:/usr/obj/usr/src/sys/FALLOUT amd64 [gcooper@fallout ~]$ stty size 60 156 [gcooper@fallout ~]$ exit Connection to fallout.local closed. [gcooper@bayonetta ~]$ uname -a FreeBSD bayonetta.local 9.0-BETA2 FreeBSD 9.0-BETA2 #0 r225653M: Tue Sep 20 08:36:49 PDT 2011 gcooper@bayonetta.local:/usr/obj/usr/src/sys/BAYONETTA amd64 [gcooper@bayonetta ~]$ stty size 60 156 # Line was wrapping in above uname -a. [gcooper@bayonetta ~]$ ssh starr-wireless # Increased the window size here from the default. starr:~ gcooper$ uname -a Darwin starr.local 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun 7 16:33:36 PDT 2011; root:xnu-1504.15.3~1/RELEASE_I386 i386 starr:~ gcooper$ echo $TERM xterm starr:~ gcooper$ logout Connection to starr-wireless.local closed. [gcooper@bayonetta ~]$ uname -a FreeBSD bayonetta.local 9.0-BETA2 FreeBSD 9.0-BETA2 #0 r225653M: Tue Sep 20 08:36:49 PDT 2011 gcooper@bayonetta.local:/usr/obj/usr/src/sys/BAYONETTA amd64 ssh'ing in to a remote terminal and resizing it is an example I could think of that's semi-deterministic. It seems like it's an application bug or OS caveat; I'm not sure if anything can really be done about it because the signal might be masked in ssh when it connects to the other side -- would have to check to be sure. By default, bash only updates its internal idea of the window size (and the LINES and COLUMNS environment variables) when it receives SIGWINCH, so only if bash is in the foreground when the change happens. You can do shopt -s checkwinsize to make it check more often. libedit (as used in sh) is different; it appears to check the size before reading each line. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: portsnap5 problem, portsnap error handling
On Tue, Oct 04, 2011 at 01:32:45PM +0300, Andriy Gapon wrote:
Not sure which list would be best for this, so using current@.
$ portsnap fetch portsnap update
Looking up portsnap.FreeBSD.org mirrors... 5 mirrors found.
Fetching snapshot tag from portsnap5.FreeBSD.org... done.
Fetching snapshot metadata... fetch:
http://portsnap5.FreeBSD.org/t/c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162:
Not Found
sha256: c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162: No
such file or directory
[: !=: unexpected operator
mv: rename c1bdea4c38cc6b417dedc1d0e75727118ed2ee08c41726ee49931f2c99288162 to
tINDEX.new: No such file or directory
done.
grep: tINDEX.new: No such file or directory
look: tINDEX.new: No such file or directory
Portsnap metadata appears bogus.
Cowardly refusing to proceed any further.
First, it seems that portsnap5 host is missing at least one important file.
Second, it seems that portsnap should detect this kind of problem a little bit
earlier. No need to call sha256, mv, etc if fetch clearly failed.
(Not sure, perhaps this is fetch not returning an error code).
The important part is the error from [. Because the check is for
inequality, in case of a [ syntax error the equal path is taken and
the script continues as if everything is fine.
The script arrives there because of a missing backslash so that the
fetch(1) command's exit status is not checked.
The below patch should fix this fairly simply. More paranoid people will
probably want to test(1) hashes for equality rather than for inequality
so that a test(1) syntax error will fail safely.
Index: usr.sbin/portsnap/portsnap/portsnap.sh
===
--- usr.sbin/portsnap/portsnap/portsnap.sh (revision 225917)
+++ usr.sbin/portsnap/portsnap/portsnap.sh (working copy)
@@ -536,9 +536,9 @@
rm -f ${SNAPSHOTHASH} tINDEX.new
echo ${NDEBUG} Fetching snapshot metadata...
- fetch ${QUIETFLAG} http://${SERVERNAME}/t/${SNAPSHOTHASH}
+ fetch ${QUIETFLAG} http://${SERVERNAME}/t/${SNAPSHOTHASH} \
2${QUIETREDIR} || return
- if [ `${SHA256} -q ${SNAPSHOTHASH}` != ${SNAPSHOTHASH} ]; then
+ if [ `${SHA256} -q ${SNAPSHOTHASH}` != ${SNAPSHOTHASH} ]; then
echo snapshot metadata corrupt.
return 1
fi
@@ -606,7 +606,7 @@
# Verify a list of files
fetch_snapshot_verify() {
while read F; do
- if [ `gunzip -c snap/${F} | ${SHA256} -q` != ${F} ]; then
+ if [ `gunzip -c snap/${F} | ${SHA256} -q` != ${F} ]; then
echo snapshot corrupt.
return 1
fi
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: cvsup broken on amd64?
On Mon, Oct 03, 2011 at 06:15:41PM +0200, Maxime Henrion wrote:
Knowing all that, what's happening seems quite clear. If
fixups_close() is called while there was still fixup requests pending,
those should be processed by the detailer thread before it returns.
Subsequent fixups_get() call should continue returning pending items,
until there are no more entries in the queue _and_ the queue is
closed.
So, line 144 in fixups.c (in fixups_get()):
if (f-closed) {
should actually be:
if (f-closed f-size == 0) {
That looks sensible.
The fact that this could even happen smells a bit weird to me though;
it means the detailer thread didn't get a chance to run between some
item was added to the queue (fixups_put() signals the detailer thread
via pthread_cond_signal()), and that it only runs now that the updater
queue has been closed. I wouldn't go as far as saying this is a bug,
but is it even valid for the pthread library to coalesce two
pthread_cond_signal() calls into one when the thread didn't get to run
since the first call was made? If so, then the above fix is perfectly
valid. If not, there is a deeper bug in the threading library, or in
the CVS mode code which I didn't write (but that seems far-fetched).
The pthread library is free to coalesce pthread_cond_signal() calls
like that. This is because a thread awakened by pthread_cond_signal()
(or any other event) is not guaranteed to start running immediately and
pthread_cond_signal() does nothing if there is no thread to wake up.
If there is no second CPU core available to run the detailer thread, it
is more efficient to have the updater thread do some more work before
incurring a context switch.
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Segfault in libthr.so on 9.0-BETA2 (with stunnel FWIW)
On Wed, Sep 14, 2011 at 11:04:56PM +0300, Kostik Belousov wrote:
tzload() allocates ~80KB for the local variables. The backtrace you provided
shows the nested call to tzload(), so there is total 160KB of the stack
space consumed.
By default, stack for the amd64 thread is 4MB, that should be plenty. This
is not the case for ezm3. Possibly, stunnel also reduces the size of the
thread stack.
Please, try the patch below. I did not tested it, only compiled. I see
that now tzload allocates only ~300 bytes on the stack.
80KB seems quite a lot indeed, good to bring it down.
diff --git a/contrib/tzcode/stdtime/localtime.c
b/contrib/tzcode/stdtime/localtime.c
index 80b70ac..55d55e0 100644
--- a/contrib/tzcode/stdtime/localtime.c
+++ b/contrib/tzcode/stdtime/localtime.c
[snip]
@@ -406,16 +409,24 @@ register const int doextend;
** to hold the longest file name string that the implementation
** guarantees can be opened.
*/
- charfullname[FILENAME_MAX + 1];
+ char*fullname;
+
+ fullname = malloc(FILENAME_MAX + 1);
+ if (fullname == NULL)
+ goto out;
if (name[0] == ':')
++name;
doaccess = name[0] == '/';
if (!doaccess) {
- if ((p = TZDIR) == NULL)
+ if ((p = TZDIR) == NULL) {
+ free(fullname);
return -1;
- if ((strlen(p) + 1 + strlen(name) + 1) = sizeof
fullname)
+ }
+ if ((strlen(p) + 1 + strlen(name) + 1) = sizeof
fullname) {
This sizeof is now the sizeof of a pointer. The comparison should be
against FILENAME_MAX + 1 instead.
Alternatively, the name could be created using asprintf().
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Serial Port Configuration does not work
On Tue, Sep 06, 2011 at 04:29:51PM +0400, Boris Samorodov wrote:
the port does not work as expected (at least as per The Handbook,
26.2.5 Serial Port Configuration). Nether init nor lock
devices can be used:
-
# uname -a
FreeBSD host1.ipt.ru 9.0-BETA2 FreeBSD 9.0-BETA2 #14 r225395: Mon Sep 5
18:10:43 MSK 2011 b...@bb.ipt.ru:/usr/obj/usr/src/sys/HOSTS i386
# ls -l /dev/ttyu5*
crw--- 1 root wheel0, 56 Sep 5 18:50 /dev/ttyu5
crw--- 1 root wheel0, 57 Sep 5 18:50 /dev/ttyu5.init
crw--- 1 root wheel0, 58 Sep 5 18:50 /dev/ttyu5.lock
# stty -f /dev/ttyu5.init 57600
stty: /dev/ttyu5.lock isn't a terminal
# stty -f /dev/ttyu5.lock cs7
stty: /dev/ttyu5.lock isn't a terminal
-
It looks like r223722, while introducing the ability to issue
device-specific ioctls on init and lock devices, broke the ability to
issue generic ioctls, with the exception of TIOCSETA. (However, stty
will not do much if it cannot do TIOCGETA.)
Try this patch and report if it works:
diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index ce49f97..3721888 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -777,6 +777,7 @@ ttyil_ioctl(struct cdev *dev, u_long cmd, caddr_t data, int
fflag,
error = ttydevsw_cioctl(tp, dev2unit(dev), cmd, data, td);
if (error != ENOIOCTL)
goto done;
+ error = 0;
switch (cmd) {
case TIOCGETA:
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: any place to look at for PCI-express performance issues ?
On Sat, Jun 11, 2011 at 02:41:50AM +0200, Luigi Rizzo wrote: just for the records: the AMD motherboard works fine and can reach 14.88Mpps, i was just doing a couple of mistakes in my AMD tests, including the use of a slot with 16x form factor but only 4 lanes connected. This said, the i7-870 is about twice as fast as the Athlon II X4-635 in generating packets for the same clock speed. I think the different cache size might have some impact on the result given the Athlon has no L3 cache and the test program surely overflows the 512k L2 cache (i am using a total of 8k packet buffers, touching 64 bytes each for the payload, plus 24 bytes each for descriptors). Unfortunately at these speeds even small things matter a lot! It may help to use non-temporal stores to fill the packet buffers. Because this data will never be read again by the CPU, caching it is useless. Also, non-temporal stores may help avoid reading a cache line only to overwrite it completely. With SSE, this could be done with a loop of four MOVUPS and four MOVNTPS instructions, transferring 64 bytes per iteration, and an SFENCE at the end (or the corresponding intrinsics from xmmintrin.h, _mm_loadu_ps(), _mm_stream_ps(), _mm_sfence()). For the receive side, there are also various non-temporal loads and prefetch instructions. On the other hand, because generating small packets only writes to 64 bytes of each 2048 byte aligned block, only a small portion of the cache will be polluted. This is because caches are usually not fully associative. This small portion could contain other important data, however. When generating full 1500 byte packets, most of the cache will be polluted. Because caching is not useful for the ring buffers, it is probably not a problem that they are laid out in such a way that they cannot be cached efficiently. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [RFC] rcexecr: rcorder in parallel
On Sat, Jun 04, 2011 at 12:45:37PM +0100, Julien Laffaye wrote: On Sat, Jun 4, 2011 at 10:10 AM, Buganini bugan...@gmail.com wrote: https://github.com/buganini/rcexecr Currently it is able to determine the exec/wait order There are something I haven't digged in deeply in the self modification part. patches/ideas are welcome. Thanks for doing that! Yes. You should use kqueue(2) instead of waitpid(2) so that you can efficiently monitor a pool of processes. See pwait(1) for an example. Hmm, I don't think kqueue() should be used here. Its main advantage is that it works regardless of parent-child relationships, but that advantage is not relevant here. On the other hand, waitpid() is still necessary to get rid of the zombies. Furthermore, waitpid() is standard while kqueue() is not, and I think non-standard interfaces should only be used if they provide a real benefit above standard interfaces. The current approach with waitpid() for specific processes should be good enough for a proof of concept. It will keep zombies longer than necessary, particularly for things that are not explicitly depended on. To avoid this, use waitpid(-1, ...) and maintain more tracking for processes that have already terminated. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: CFR: importing openresolv
On Tue, Feb 22, 2011 at 02:51:06AM +0900, Hajimu UMEMOTO wrote:
On Tue, 22 Feb 2011 01:50:17 +0900
Hajimu UMEMOTO u...@freebsd.org said:
bz Do you have an updated patch for 3.4.1 or 3.3.6? I'd like to help to
bz you get it in for 9.0-R. I wouldn't even mind if some ports would
bz conflict with it for a while not making the situation any worse than
bz it is these days.
ume I didn't notice that openresolv was updated. I'll soon make a new
ume diff for 3.4.1.
ume hrs@ noticed that ppp(8) and uhsoctl(8) in our base tree touch
ume /etc/resolv.conf. We need to change them to use resovlconf(8).
I've updated a patch for 3.4.1:
http://www.imasy.or.jp/~ume/FreeBSD/openresolv-20110222.diff.gz
If you have the previous patch applied, make sure to remove
src/contrib/openresolv and src/sbin/resolvconf before applying new
one.
This looks like it can work, but note that it depends on some 9-CURRENT
sh(1) features:
* A printf(1) builtin, required to run without /usr. I don't see a sane
way to avoid this.
* Some of the expansion changes, required for the line:
iface=${line#\# resolv.conf from *}
This can be avoided by changing that line to:
iface=${line#\# resolv.conf from *}
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [head tinderbox] failure on ia64/ia64
On Tue, Feb 01, 2011 at 09:18:44AM +0100, Pawel Jakub Dawidek wrote: Still, I'm more concerned with CMSG_NXTHDR() macro, which from what I see might not be fixed by casting arguments. Yes, without various checks it expands to (struct cmsghdr *)((char *)(cmsg) + \ _ALIGN(((struct cmsghdr *)(cmsg))-cmsg_len)) Although there is no alignment problem (assuming cmsg is properly aligned and _ALIGN is correct), this violates -Wcast-align. Therefore I think an intermediate cast to void * would be appropriate here. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: FYI: clang static analyzer page has moved to http://scan.freebsd.your.org/freebsd-head/
On Wed, Jan 05, 2011 at 10:30:43PM +0100, Ulrich Spörlein wrote: On Wed, 05.01.2011 at 20:36:53 +0100, Jilles Tjoelker wrote: On Wed, Jan 05, 2011 at 05:55:45PM +0100, Ulrich Spörlein wrote: *But*, it should grok that for err(3) and exit(3). Now there are some possible remedies: - get IPA to work with clang, or at least file a bug - mark functions as __dead2 (please don't do that) Why not? Cause IMHO it adds clutter, is noisy and needs to be maintained manually, when we have these computer things that should deduct this by themselves. Yes, but to me it seems the only realistic option of your three. Upstream is unlikely to add IPA to the checker and other kinds of annotation are probably either similar to __dead2 with the same problems and an additional one that gcc does not check it or very specific to a particular complaint from the checker. I have done this in some cases because it leads to better code with gcc (the system version in 9-current). See SVN commit r212508 to bin/sh/parser.c. Although synexpect() and synerror() are static, adding __dead2 to both makes the executable 576 bytes smaller on i386 (these functions are called many times). Adding __dead2 to synexpect() only causes a warning noreturn function does return (it calls synerror()). Adding __dead2 to synerror() only also makes the executable smaller but not as much as adding it to both. Reordering the functions in the file does not help to make gcc see that the functions do not return. This is too bad and really makes me sad. It shouldn't be necessary to hand-hold the compilers like that. Could you try some tests with gcc 4.5 to confirm this is still required? gcc 4.5 still needs it. gcc 4.6 and clang (the compiler) do not need it. (For gcc, used ports gcc and compiled head bin/sh with some patches on stable/8. For clang, used base clang and compiled head bin/sh on head.) -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: FYI: clang static analyzer page has moved to http://scan.freebsd.your.org/freebsd-head/
On Wed, Jan 05, 2011 at 05:55:45PM +0100, Ulrich Spörlein wrote: On Wed, 05.01.2011 at 09:34:49 -0500, John Baldwin wrote: These are all marked as __dead2, so the compiler should know that these do not return. And clang did the right thing here in the past. Beware that it does no inter-procedural analysis yet, so it will usually miss that usage() calls exit unconditionally. *But*, it should grok that for err(3) and exit(3). Now there are some possible remedies: - get IPA to work with clang, or at least file a bug - mark functions as __dead2 (please don't do that) Why not? I have done this in some cases because it leads to better code with gcc (the system version in 9-current). See SVN commit r212508 to bin/sh/parser.c. Although synexpect() and synerror() are static, adding __dead2 to both makes the executable 576 bytes smaller on i386 (these functions are called many times). Adding __dead2 to synexpect() only causes a warning noreturn function does return (it calls synerror()). Adding __dead2 to synerror() only also makes the executable smaller but not as much as adding it to both. Reordering the functions in the file does not help to make gcc see that the functions do not return. - come up with a way to mark the false positives (kinda impossible with the way scan-build currently works) -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: head/bin/sh/output.h r215303
On Sun, Nov 14, 2010 at 11:36:45AM -0500, jhell wrote: I just merged some of the changes to stable/8 of bin/sh, specifically all the recent changes that happened. In revision 215303 it shows that you added stddef.h as an include to output.h while jobs.c includes both output.h and stddef.h resulting in 'offsetof' being redefined. This problem was introduced by r213775 include changes, which you have merged yourself. I had already noticed this myself in head and fixed it in r213925. I have now MFCed r213775 and r213925 so your problem should be solved. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: Files under src/ not used for building world
On Fri, Nov 05, 2010 at 09:42:27PM -0700, Garrett Cooper wrote: On Fri, Nov 5, 2010 at 1:25 PM, Ulrich Spörlein u...@spoerlein.net wrote: Hey folks, not sure why, but I had a stab at looking which files were actually read during building world. bin/sh/bltin/echo.1 I'd talk to jilles@ (weird thing is that this is installed on my machine). This is a man page for sh's echo builtin. It is a little more extensive but otherwise equivalent to the description in sh(1). It is different from bin/echo/echo.1. Unfortunately, sh's echo builtin and /bin/echo work differently. This should not have been allowed to happen, but now we are stuck with it. I suppose bin/sh/bltin/echo.1 can go away. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: [PATCH] fix shell bug in ${var%pattern} expansion
On Mon, Oct 11, 2010 at 07:19:14PM -0700, David O'Brien wrote:
At $WORK we hit a bug where ${var%/*} was not producing the correct
expansion. There are two patches to fix this. I prefer the first
as I feel it is cleaner from an API perspective. I've also added
a regression testcase that shows the problem.
Thoughts?
Thank you, I think this is the mysterious bug I encountered a while ago
while making changes to arithmetic expansion, heavily using the stack
string. It only failed when /etc/rc.d/nfsserver was called from /etc/rc
on boot.
Your patch looks good (the first one) and works for me. The second patch
indeed seems a hack rather than fixing the problem properly.
Style bug:
+growstrstackblock(int n) {
The opening brace should be on its own line.
Your test is too fragile: it often fails to detect the bug. Calling like
sh -c '. expansion/trim4.0'
gives the correct output even with a buggy sh. I propose something like
this, or perhaps with an additional string comparison:
# $FreeBSD$
v1=/homes/SOME_USER
v2=
v3=C123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
while [ ${#v2} -lt 2000 ]; do
v4=${v2} ${v1%/*} $v3
if [ ${#v4} -ne $((${#v2} + ${#v3} + 8)) ]; then
echo bad: ${#v4} -ne $((${#v2} + ${#v3} + 8))
fi
v2=x$v2
v3=y$v3
done
--
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
Re: CFR: Replace man/manpath/whatis/apropos with a shell script
On Thu, Sep 09, 2010 at 11:48:37PM +0400, Anonymous wrote: Gordon Tetlow gor...@freebsd.org writes: Gordon Tetlow gor...@freebsd.org writes: Anonymous swel...@gmail.com writes: It doesn't search in bin/../man nor in bin/.man. For example, my PATH contains $LOCALBASE/bin:$HOME/.bin, while /etc/ manpath.config is default one and contains /usr/local/man which does not exist here. Guess I missed that pretty badly in my port. I'll go back and retool the logic for this but that'll take a bit of time. Added. Latest version at http://people.freebsd.org/~gordon/man.sh The order is still bogus compared to gnu man. If I don't like our ancient GNU tools and altered PATH in order to prefer ones from ports then I certainly don't want to view old manpages, too. The base manpath should be appended *after* any PATH substitutions. That is appropriate, but to avoid breaking the more common setup with /usr/bin before /usr/local/bin, search_path needs to map the PATH directories /bin and /usr/bin to the man directory /usr/share/man. GNU man does the same, but it is written into /etc/manpath.config. -- Jilles Tjoelker ___ freebsd-current@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-current To unsubscribe, send any mail to freebsd-current-unsubscr...@freebsd.org
