| Message ID | 20210223110640.73974-1-akihiko.odaki@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ui/cocoa: Do not rely on the first argument | expand |
On Tue, Feb 23, 2021 at 08:06:40PM +0900, Akihiko Odaki wrote: > The first argument of the executable was used to get its path, but it is > not reliable because the executer can specify any arbitrary string. Use the > interfaces provided by QEMU and the platform to get those paths. > > This change also changes the icon shown in the "about" window to QEMU's > cute one. Looks good to me, but I'd like to have an additional macos review for this one. thanks, Gerd
On Tue, 23 Feb 2021 at 11:06, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > > The first argument of the executable was used to get its path, but it is > not reliable because the executer can specify any arbitrary string. Use the > interfaces provided by QEMU and the platform to get those paths. > > This change also changes the icon shown in the "about" window to QEMU's > cute one. This patch seems to be doing two things at once: * we get the executable path via the OSX APIs rather than using argv[0] * we get an image from the icon directory rather than using the executable's built-in icon Please don't put more than one change in the same patch: it makes it harder to review. Could you split this up so that there is one patch per change (ie make it a 2-patch series), please ? > @@ -1401,18 +1402,13 @@ - (void)make_about_window > y = about_height - picture_height - 10; > NSRect picture_rect = NSMakeRect(x, y, picture_width, picture_height); > > - /* Get the path to the QEMU binary */ > - NSString *binary_name = [NSString stringWithCString: gArgv[0] > - encoding: NSASCIIStringEncoding]; > - binary_name = [binary_name lastPathComponent]; > - NSString *program_path = [[NSString alloc] initWithFormat: @"%@/%@", > - [[NSBundle mainBundle] bundlePath], binary_name]; > - > /* Make the picture of QEMU */ > NSImageView *picture_view = [[NSImageView alloc] initWithFrame: > picture_rect]; > - NSImage *qemu_image = [[NSWorkspace sharedWorkspace] iconForFile: > - program_path]; > + char *qemu_image_path_c = get_relocated_path(CONFIG_QEMU_ICONDIR "/hicolor/512x512/apps/qemu.png"); > + NSString *qemu_image_path = [NSString stringWithUTF8String:qemu_image_path_c]; > + g_free(qemu_image_path_c); > + NSImage *qemu_image = [[NSImage alloc] initWithContentsOfFile:qemu_image_path]; > [picture_view setImage: qemu_image]; > [picture_view setImageScaling: NSImageScaleProportionallyUpOrDown]; > [superView addSubview: picture_view]; > @@ -1427,9 +1423,7 @@ - (void)make_about_window > [name_label setBezeled: NO]; > [name_label setDrawsBackground: NO]; > [name_label setAlignment: NSTextAlignmentCenter]; > - NSString *qemu_name = [[NSString alloc] initWithCString: gArgv[0] > - encoding: NSASCIIStringEncoding]; > - qemu_name = [qemu_name lastPathComponent]; > + NSString *qemu_name = [[[NSBundle mainBundle] executablePath] lastPathComponent]; The API docs for NSBundle mainBundle say it can return nil and you always have to check the return value. https://developer.apple.com/documentation/foundation/nsbundle/1410786-mainbundle It's not clear to me what the fallback should be if it does return nil... > [name_label setStringValue: qemu_name]; > [superView addSubview: name_label]; > > -- > 2.24.3 (Apple Git-128) thanks -- PMM
diff --git a/ui/cocoa.m b/ui/cocoa.m index 0ef5fdf3b7a..b3e7833e7fa 100644 --- a/ui/cocoa.m +++ b/ui/cocoa.m @@ -39,6 +39,7 @@ #include "qapi/qapi-commands-misc.h" #include "sysemu/blockdev.h" #include "qemu-version.h" +#include "qemu/cutils.h" #include "qemu/main-loop.h" #include "qemu/module.h" #include <Carbon/Carbon.h> @@ -1401,18 +1402,13 @@ - (void)make_about_window y = about_height - picture_height - 10; NSRect picture_rect = NSMakeRect(x, y, picture_width, picture_height); - /* Get the path to the QEMU binary */ - NSString *binary_name = [NSString stringWithCString: gArgv[0] - encoding: NSASCIIStringEncoding]; - binary_name = [binary_name lastPathComponent]; - NSString *program_path = [[NSString alloc] initWithFormat: @"%@/%@", - [[NSBundle mainBundle] bundlePath], binary_name]; - /* Make the picture of QEMU */ NSImageView *picture_view = [[NSImageView alloc] initWithFrame: picture_rect]; - NSImage *qemu_image = [[NSWorkspace sharedWorkspace] iconForFile: - program_path]; + char *qemu_image_path_c = get_relocated_path(CONFIG_QEMU_ICONDIR "/hicolor/512x512/apps/qemu.png"); + NSString *qemu_image_path = [NSString stringWithUTF8String:qemu_image_path_c]; + g_free(qemu_image_path_c); + NSImage *qemu_image = [[NSImage alloc] initWithContentsOfFile:qemu_image_path]; [picture_view setImage: qemu_image]; [picture_view setImageScaling: NSImageScaleProportionallyUpOrDown]; [superView addSubview: picture_view]; @@ -1427,9 +1423,7 @@ - (void)make_about_window [name_label setBezeled: NO]; [name_label setDrawsBackground: NO]; [name_label setAlignment: NSTextAlignmentCenter]; - NSString *qemu_name = [[NSString alloc] initWithCString: gArgv[0] - encoding: NSASCIIStringEncoding]; - qemu_name = [qemu_name lastPathComponent]; + NSString *qemu_name = [[[NSBundle mainBundle] executablePath] lastPathComponent]; [name_label setStringValue: qemu_name]; [superView addSubview: name_label];
The first argument of the executable was used to get its path, but it is not reliable because the executer can specify any arbitrary string. Use the interfaces provided by QEMU and the platform to get those paths. This change also changes the icon shown in the "about" window to QEMU's cute one. Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- ui/cocoa.m | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)