In D19902#437991, @hrs wrote:

In D19902#437962, @jilles wrote:

Perhaps it is cleaner to pass the int age to be searched for by reference. However, the asymmetry in the compare function is intentional as this allows finding an object with more data given the key, while having only one storage array.

I think it will be useful to have this. It fixes most of the drop in functionality when moving from System V shared memory to POSIX shared memory.

In D19902#437950, @hrs wrote:

As already pointed out on the mailing list, this example has a lot of problems which are not only style bugs. Ones I think as critical except for the style issues are the following:

• The function to compare two elements should accept the same type, not using an "age" field value by using a pointer type. It is a function to compare, not look up a matched element in the definition of bsearch(3). Passing a raw value as (void *) type is not impossible, but it is not suitable for a "textbook" example.

The new function sbuf_printf_drain was added recently enough that this should be safe (binaries using sbuf_printf_drain linked against the old libsbuf.so.6 will not run against the new libsbuf.so.6).

In D20111#433337, @asomers wrote:

In D20111#433326, @jilles wrote:

In D20111#432908, @asomers wrote:

Not so fast! I just discovered a serious footgun: Linux's struct dirent has a
d_off field, but it's meaning is different than FreeBSD's. That's worth
mentioning.

In D20111#432908, @asomers wrote:

Not so fast! I just discovered a serious footgun: Linux's struct dirent has a
d_off field, but it's meaning is different than FreeBSD's. That's worth
mentioning.

It is indeed important to ensure that this is the complete kernel assist for posix_spawn, so we do not leave multiple versions of this around, each with their own flag.

The general approach seems good.

This seems a good approach to make things work. Malloc typically uses mutexes, so it makes things much easier if mutexes do not use malloc.

In D18926#406366, @se wrote:

I'd do not like the conversion to floating point for numbers outside the range of 64 bit integers. They violate assumptions I'd like to be able to make (e.g. that adding and then subtracting the same number leads back to the initial value in all cases).

Add test for [ERANGE] special case.

In D18926#405742, @se wrote:

The behavior is not consistent for shell arithmetic operating on parameters with or without "$" (e.g. "$((X))" vs. "$(($X))") for variables not in the range of valid integers.
This may be allowed by the standard, but is still somewhat surprising:

My original objection to \u is no longer valid since we have accepted that the ugly \ bash prompt sequences are staying.

The BLOCKSIZE assignments in share/skel are not "wrong" as such, but BLOCKSIZE seems a configuration option of little use in general. Most of the time, the -h option is more useful.

I wonder when it is appropriate to run pwd_mkdb without -p. Only when etc/passwd is considered legacy and needs to be omitted from the system?

If this is only for mounting filesystems, starting mirror/raid and the like, then the new error makes sense to me, including mapping it to EINVAL in the Linux compatibility layer.

A case could be made that it is really login(1) and sshd(8)'s job to set PWD along with chdir(2), but this will do.

In D18765#400410, @mckusick wrote:

Mirror might be able to read both sides but find that they have different values. So it would not know which one to return. Here EIO is not really the right answer as the media is fine, it is just the data that is inconsistent. So saying EINTEGRITY gives a hint that a higher level tool might be able to figure out which one was right and correct the wrong one. None of that is done today, but it is conceivable that it could be done. That said, my main intent is to return this error from UFS when it detects inconsistencies or when a check-hash fails.

A new test in bin/dd/tests that does dd if=/dev/null of=/dev/null status=none conv=trim would be nice (by not writing/trimming any bytes, it does not need a fake disk setup, but it does verify that the option is recognized). Even better would be something using md but that would need more ATF/Kyua magic.

It seems like the idea is that we're stuck with that ugly \h/\$ bashism anyway so we might as well use it.

A world-readable pidfile can be locked in shared mode by any user, which may cause the real daemon to act as if it is already running when it is not (e.g. after it crashed).

"Cookie" or "handle" are definitely good words here. The part "to position the directory descriptor to the start of the entry next to the current" could be simplified a bit to "to position the directory descriptor to the next entry" since the position cannot be in the middle of an entry.

The man page seems to contain the right content now. I just have some English language comments.

In D17547#375528, @kib wrote:

In D17547#375474, @jilles wrote:

Unconditionally allowing absolute paths with O_BENEATH seems a bit strange, as I expect this to be used for partially untrusted paths. An application can easily check for absolute paths and cause either behaviour (allowing or disallowing).

This was one of my questions. But, it is not clear to me what change, if any, you are proposing.

OUTBUFSIZ is somewhat of a strange name given that these buffers contain shell input.

I think it would make more sense to define a new buffer size constant than modifying BUFSIZ.

Unconditionally allowing absolute paths with O_BENEATH seems a bit strange, as I expect this to be used for partially untrusted paths. An application can easily check for absolute paths and cause either behaviour (allowing or disallowing).

I don't think "Show time for sorting" is better than "Use time for sorting". If you want to avoid the weak "use", something like "Sort by (with .Fl t) or write (with .Fl l) time when file ..." may be better.

This is not wrong, but perhaps some details about different types of errors may be helpful, such as that nonexistent usernames in -U are immediately fatal while an empty result (such as from only nonexistent process IDs in -p or only existing users that do not currently run any process in -U) still writes the header line (if not disabled) and also returns a non-zero exit status.

I would like to see a consistent behaviour in handling chown/chmod failures between creating directories and installing files. A -e flag looks ugly and should only be added if both behaviours are necessary in different situations.

There is actually another important difference between reboot and shutdown -r now: reboot does not run /etc/rc.shutdown. This is because reboot has its own shutdown procedure and does not signal init like init 6 and shutdown -r now do (except in the case of rerooting via reboot -r).