In D48244#1102240, @emaste wrote:

But what possible value is there today in making it easy to find that e.g. sigset_t's type changed in September 1999? I can see an argument for keeping recently-out-of-support versions (e.g. 12.x) as some downstreams might still be using them.

In D48331#1102324, @imp wrote:

In D48331#1102305, @markj wrote:

In D48331#1102183, @imp wrote:

Why 5, but regardless of why, this is correct

A bit further down there's an assignment p -= 5.

Where does that 5 come from was my real question. What spec says we know this format

It seems to be common in other cases to print the -ve values under else if (verbose), does that make sense here?

Approved.

Commited in https://cgit.freebsd.org/ports/commit/?id=2cf6bc1add649b35668b449ab35fb04ed0a6d263

In D48331#1102305, @markj wrote:

In D48331#1102183, @imp wrote:

Why 5, but regardless of why, this is correct

A bit further down there's an assignment p -= 5.

In D48331#1102305, @markj wrote:

In D48331#1102183, @imp wrote:

Why 5, but regardless of why, this is correct

A bit further down there's an assignment p -= 5.

LGTM as well and I also agree with @0mp suggestion for commit log

This broadly looks good to me. I'd suggest having a comment somewhere which explains at a higher level what the tests are testing.

In D48331#1102183, @imp wrote:

Why 5, but regardless of why, this is correct

Please make sure to add PR: 283364 to the commit message.

At least tests/sys/file/path_test.c:path_io will need to be updated.

@kiwi Just to let you know that I have updated the ports in the meantime. So make sure to rebase. And I think you don't the need the portlint/portclippy part anymore.

Added - Pet portlint/portclippy as asked

In D48335#1102258, @emaste wrote:

What takes the place of the now-removed locking though?

Remove the todo comment from riscv as well.

Regarding the commit message:

I think this is OK. But it would also be fine just to put an assert(j < max_queue_per_proc before the array dereference.

And D48336 for the multiple primary channels issue. Closing this.

What takes the place of the now-removed locking though?

I've got some suggestions for the commit message:

In D47918#1100261, @christos wrote:

In D47918#1100168, @dev_submerge.ch wrote:

In D47918#1100131, @christos wrote:

In D47918#1099644, @dev_submerge.ch wrote:

Not a consequence of your changes here, but it seems we only set the vchan settings on the first channel per direction. There could be multiple, right?

IIRC, there's only one or two sound drivers that support multiple channels per direction. Maybe we should evaluate whether that's actually required and worth the complication it causes throughout the sound module.

There are more places in pcm/ where we always pick the first primary channel. I guess we could implement some heuristics, like choosing the first non-busy channel, or else fall back to the first channel, or the channel with the least vchans, in order to balance things out, but that would require multiple changes throughout sound(4), so we could discuss this further in an email.

Just to clarify, my general proposal would be to see whether we can limit all drivers to at most one primary channel per direction. If that's possible, the primary channel list reduces to two fields (one per direction), and handling them should become easier in the places you mention. It's a separate discussion though, agreed.

My point regarding the code here is that we change settings for only one of the primary channels, while we should do this for all primary channels to keep them consistent. Which is an example of where it would be easier to have only one primary channel per direction.

I guess one way to go about addressing this would be to actually get rid of vchan_getparentchannel() altogether, and instead wrap the related sysctl codes around a loop through the primary channel list (see D47917). For sysctl_dev_pcm_vchanrate() and sysctl_dev_pcm_vchanformat() this is very trivial to implement. For sysctl_dev_pcm_vchanmode() it's probably trickier, because the code seems to fetch the mode from the primary channel directly; maybe this is something we should store in snddev_info instead, like vchanformat/vchanrate?

Regarding the commit message, I'd suggest something along the lines of:

In D48244#1101534, @mat wrote:

I am not sure this is a good idea.

This list is the only definitive list of "this version corresponds to that change".

Address all remaining comments.

This is a follow-up to https://reviews.freebsd.org/D48190, where it was discovered that this support is missing. It's obvious that our test suite does not use it, and this patch is for consistency and not to say "not mapped" in the kyuafile.5. Also, I found that Kyua 0.11 release notes state that it should work, so actually this is a fix.

In D48190#1100132, @ngie wrote:

I think submitting this upstream (freebsd/kyua) and filing an issue capturing the feedback/concerns would probably be ok.

In D48087#1096440, @kp wrote:

One trivial remark about 'pip? # e.g. to have scapy installed'. We have a scapy package and that's how I've always installed it. Right now it's py311-scapy-2.6.1.