Re: [PATCH v2] ui/cocoa: Create menus in iothread

[Peter Maydell]

On Sun, 6 Mar 2022 at 06:57, Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
>
> Commit 0439c5a4623d674efa0c72abd62ca6e98bb7cf87 introduced an
> assertion that blk_all_next is called in the main thread. The function
> is called in the following chain:
> - blk_all_next
> - qmp_query_block
> - addRemovableDevicesMenuItems
> - main
>
> This change moves the menu creation to the iothread. This also changes
> the menu creation procedure to construct the entire menu tree before
> setting to NSApp, which is necessary because a menu set once cannot be
> modified if NSApp is already running.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  ui/cocoa.m | 209 +++++++++++++++++++++++++----------------------------
>  1 file changed, 98 insertions(+), 111 deletions(-)
>
> diff --git a/ui/cocoa.m b/ui/cocoa.m
> index 8ab9ab5e84d..edacbef9f7a 100644
> --- a/ui/cocoa.m
> +++ b/ui/cocoa.m
> @@ -1604,97 +1604,6 @@ - (void)sendEvent:(NSEvent *)event
>  }
>  @end
>
> -static void create_initial_menus(void)
> -{
> -    // Add menus
> -    NSMenu      *menu;
> -    NSMenuItem  *menuItem;
> -
> -    [NSApp setMainMenu:[[NSMenu alloc] init]];
> -    [NSApp setServicesMenu:[[NSMenu alloc] initWithTitle:@"Services"]];
> -
> -    // Application menu



> +static void create_menus(void)
> +{
> +    // Add menus
> +    NSString    *title = [[[NSBundle mainBundle] executablePath] 
> lastPathComponent];
> +    NSMenu      *mainMenu;
> +    NSMenu      *menu;
> +    NSMenuItem  *menuItem;
> +
> +    mainMenu = [[NSMenu alloc] initWithTitle:@"Main Menu"];
> +    [NSApp setServicesMenu:[[NSMenu alloc] initWithTitle:@"Services"]];
> +
> +    // Application menu

Why have you moved this function around in the source file ?
It makes the patch harder to review. If you need to do this, please
do the code-motion as a separate "this patch only moves code without
changing it" step first.

thanks
-- PMM