On 8/22/22 18:05, Rémi Bernon (@rbernon) wrote:
I'm having a hard time seeing this as an improvement.
Gitlab doesn't show any context here as the comments aren't attached to commits, so I'm guessing from the shortstat that it's about releasing the callback on async_reader_close, and I don't see how it's not an improvement.
The callback is acquired in async_reader_open, and at least for the sake of symmetry async_reader_close should undo everything the former did.
Sorry about that, I'll make it clearer which patch I'm responding to in the future.
Reading the patch a little more carefully, the callback and context part is fine and makes sense. What I'm less convinced about, and what the patch subject doesn't actually mention, is that it also moves the async_reader_queue_op() and "reader->running = false" calls from two different paths to that function, which doesn't immediately make much sense to me.