Re: [PATCH v13 2/7] net/vmnet: add vmnet backends to qapi/net

[Akihiko Odaki]

On Tue, Jan 25, 2022 at 7:32 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 25 Jan 2022 at 04:14, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> > I'm neutral about the decision. I think QEMU should avoid using
> > Objective-C code except for interactions with Apple's APIs, and .c is
> > superior in terms of that as it would prevent accidental introduction
> > of Objective-C code. On the other hand, naming them .m will allow the
> > introduction of Automatic Reference Counting to manage dispatch queue
> > objects. In fact, I have found a few memory leaks in vmnet in the last
> > review and ui/cocoa.m has a suspicious construction of the object
> > management (Particularly it has asynchronous dispatches wrapped with
> > NSAutoreleasePool, which does not make sense).
>
> I think those are probably my fault -- in commit 6e657e64cd (in 2013)
> we added NSAutoReleasePools to fix leaks that happened because
> we were calling into Cocoa APIs from threads other than the UI
> thread that didn't have their own automatically created autorelease
> pool. Much later in commit 5588840ff778 (in 2019) we put in the
> dispatch_async stuff because newer macOS was stricter about
> requiring Cocoa API calls to be only on the UI thread. So
> I think that means the requirement for the autorelease pools
> has now gone away in those functions and we could simply delete
> them -- does that sound right? (I freely admit that I'm not a macOS
> expert -- I just look stuff up in the documentation; historically
> we haven't really had many expert macOS people around to work on
> cocoa.m...)

Removing them would be an improvement. Enabling ARC is a long-term
solution and would allow clang to analyze object management code and
answer such a question semi-automatically.

Regards,
Akihiko Odaki

>
> On the subject of cocoa.m, while we have various macOS-interested
> people in this thread, can I ask if anybody would like to
> review a couple of patches that came in at the beginning of the
> year?
>
> 20220102174153.70043-1-carwynellis@gmail.com/">https://patchew.org/QEMU/20220102174153.70043-1-carwynellis@gmail.com/
> ("ui/cocoa: Add option to disable left command and hide cursor on click")
> and
> 20220103114515.24020-1-carwynellis@gmail.com/">https://patchew.org/QEMU/20220103114515.24020-1-carwynellis@gmail.com/
> ("Show/hide the menu bar in fullscreen on mouse")
>
> either from the point of view of "is this a sensible change to
> the macOS UI experience" or for the actual code changes, or both.
>
> We've been very short on upstream macOS code reviewers so if people
> interested in that host platform are able to chip in by
> reviewing each others' code that helps a lot.
>
> thanks
> -- PMM