Closed satmandu closed 2 years ago
Thanks!
I need to update the CI runner to this version for good measure.
Also sharing the reference to the upstream patch that makes this change necessary; https://lore.kernel.org/all/20220730052643.1959111-4-quic_vjakkam@quicinc.com/
@satmandu I just addressed all my comments directly on your branch, and the changes build successfully on Linux 6.1 rc1.
Summary:
I'm ready to click the merge button whenever you confirm this is also looking good on your side
FYI here is another approach which is less explicit but requires less changes: https://gist.github.com/joanbm/94323ea99eff1e1d1c51241b5b651549
I personally don't have a strong preference, being explicit is often good as well. In that particular case, I would even say it makes future changes to these function signatures easier to introduce.
I was waiting for his patch to show up!
That patch is so much simpler. I would just go with that...
I've been thinking about it since my last message, and actually I think I like your approach better.
It's not as "simple", but if anything in these function signatures changes again, Joan's patch needs to be reverted/revisited, whereas with your version it's just another if
. Less chances to break something unintentionally.
I'm also happy to see new people contribute patches and show up in the commit history, appreciate your work! 🙌 If Joan's approach turns out to bring other advantages in the future, I'm happy to revisit.
This was tested on a MacBookPro11,3.