Bug 24474

52-always-observe-new-channels (referenced as the URL for this bug) fixes this bug for 5.2.

always-observe-new-channels is a reimplementation for master; I do not recommend merging this one.

always-observe-after-refactor is a reimplementation on top of dispatcher-refactor-part4.

Fixed in 5.2. I recommend fixing this in 5.3 by merging dispatcher-refactor-part4 first: accordingly, blocking by Bug #24120.

+    /* channels that we will only observe should not need approval - so at
+     * least one must be false */
+    g_return_val_if_fail (!(observe_only && needs_approval), NULL);

Are you trying to say observe_only => !needs_approval, because the assertion is a non-obvious way of writing that and the comment doesn't make it very clear either


Assuming this still ignores channels on connections that were established behind mc5's back, the branch looks fine. (apart from probably an improved comment)

Is this better?

     /* possible-handlers is only allowed to be NULL if we're only observing */
     g_return_val_if_fail (possible_handlers != NULL || observe_only, NULL);
-    /* channels that we will only observe should not need approval - so at
-     * least one must be false */
-    g_return_val_if_fail (!(observe_only && needs_approval), NULL);
+
+    /* If we're only observing, then the channels were requested "behind MC's
+     * back", so they can't need approval (i.e. observe_only implies
+     * !needs_approval) */
+    g_return_val_if_fail (!observe_only || !needs_approval, NULL);

Reviewed by Sjoerd on IRC and fixed in git.