NewChannels with a number of channels other than one is deprecated, because in practice, no client handles it correctly. We're gradually removing this usage from connection managers. To reduce client confusion even before we've fixed all the CMs, we could modify MC to dispatch channels one at a time. This cascades into quite a lot of simplification in McdDispatchOperation etc., where lists become single nullable channels. I have a branch, but it's based on some unrelated (and quite extensive) refactoring. I'm rebasing it.
Created attachment 64431 [details] [review] McdConnection: remove virtual methods Now that embedding and old-style plugins have been abolished, nothing can override them.
Created attachment 64432 [details] [review] McdConnection: dispatch each channel from NewChannels separately This loses information - you can no longer tell the channels are related - but in practice nobody handles Text and Tube channels as a unit anyway, and signalling them together is now deprecated.
Created attachment 64433 [details] [review] _mcd_dispatcher_take_channels: make singular, rename to ...add_channel It was previously (transfer container), now it's (transfer none).
Created attachment 64436 [details] [review] mcd_dispatcher_dup_possible_handlers: only take one TpChannel
Created attachment 64437 [details] [review] _mcd_client_registry_list_possible_handlers: only take one channel
Created attachment 64438 [details] [review] _mcd_dispatcher_enter_state_machine: only take one channel
Created attachment 64439 [details] [review] McdDispatchOperation: remove untrue comment I dimly remember that _mcd_dispatch_operation_new once had (transfer container) or possibly even (transfer full) semantics for the list of channels, but I also dimly remember removing it. The implementation certainly has (transfer none) semantics, and its one caller behaves as such too.
Created attachment 64440 [details] [review] McdDispatchOperation: change channels property to channel (singular)
Created attachment 64441 [details] [review] _mcd_dispatch_operation_peek_channel: adapt from old _peek_channels Now that the CDO can contain at most one channel, there's no need to return a list.
Created attachment 64442 [details] [review] McdDispatchOperation: use internal list instead of dup_channels() Now that there can only be at most one channel in the list, it's pretty easy to do directly.
Created attachment 64443 [details] [review] _mcd_dispatch_operation_dup_channels: turn into dup_channel, singular
Created attachment 64444 [details] [review] _mcd_dispatch_operation_new: only take one channel
Created attachment 64445 [details] [review] Replace iteration over channels with check for having a channel Now that there can only be one channel, we don't need to iterate.
Created attachment 64446 [details] [review] McdDispatchOperation: collect_satisfied_requests: only take one channel
Created attachment 64447 [details] [review] McdDispatchOperation: store one McdChannel, not a list
Created attachment 64448 [details] [review] McdDispatchOperation: only have one lost_channel, too
Yeah this looks fine. I was going to comment on how there are a couple of FIXMEs added, but yeah whatever. I also didn't really review every change to the test suite — you probably got it right. Hot.
Sorry, I mean: m m m # # mmm mm#mm #mmmm# #" "# # # # # # # # # "#m#" "mm #
Fixed in master for 5.13.1, thanks.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.