qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2] Machine menu patch for Mac OS X


From: Programmingkid
Subject: Re: [Qemu-devel] [PATCH v2] Machine menu patch for Mac OS X
Date: Wed, 11 Feb 2015 23:33:33 -0500

On Feb 11, 2015, at 9:41 PM, Peter Maydell wrote:

> On 20 January 2015 at 17:22, Programmingkid <address@hidden> wrote:
>> Features:
>> Menu items to switch floppy and CD image files.
>> Menu items to eject floppy and CD image files.
>> Menu item to use /dev/cdrom.
>> Verifies with the user before quitting QEMU by displaying a dialog box.
>> 
>> Signed-off-by: John Arbuckle <address@hidden>
> 
> I get lots of compile warnings with this, including about
> functions deprecated back in OSX 10.4, format string security
> issues, and discarding of const qualifiers. These all need to
> be fixed.

We could turn off the depreciation flag when compiling. I really doubt Apple is 
going to remove these functions because it would eliminate compatibility with a 
lot of Applications. But if you really want, I guess I could use all 
non-depreciated functions. 

> 
>> 
>> int gArgc;
>> char **gArgv;
>> +#define MAX_DEVICE_NAME_SIZE 10
>> +char floppy_drive_name[MAX_DEVICE_NAME_SIZE], 
>> cdrom_drive_name[MAX_DEVICE_NAME_SIZE];
>> +NSTextField * pause_label;
> 
>> +/*
>> +Determine if the current emulator has the specified device.
>> +device_name: the name of the device you want: floppy, cd
>> +official_name: QEMU's name for the device: floppy0, ide-cd0
>> +*/
>> +static bool emulatorHasDevice(char * device_name, char * official_name)
>> +{
>> +    BlockInfoList * block_device_data;
>> +    block_device_data = qmp_query_block(false);
>> +    if(block_device_data == NULL) {
>> +        return false;
>> +    }
>> +    while(block_device_data->next != NULL) {
>> +        /* If we found the device */
>> +        if (strstr(block_device_data->value->device, device_name)) {
>> +            strcpy(official_name, block_device_data->value->device);
> 
> This is doing an unchecked strcpy into a fixed size buffer;
> don't ever do this...

Is using strncpy() in place of strcpy() ok?

> 
>> +            qapi_free_BlockInfoList(block_device_data);
>> +            return true;
>> +        }
>> +        block_device_data = block_device_data->next;
>> +    }
>> +    return false;
>> +}
> 
>> +/* Adds the Machine menu to the menu bar. */
>> +/* Has to be added separately because QEMU needs
>> +   to be running to determine used devices.
>> +*/
>> +static void createMachineMenu()
>> +{
>> +    NSMenu * menu;
>> +    NSMenuItem * menuItem;
>> +
>> +    // Machine menu
>> +     menu = [[NSMenu alloc] initWithTitle: @"Machine"];
>> +    [menu setAutoenablesItems: NO];
>> +    [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Pause" action: 
>> @selector(pauseQemu:) keyEquivalent: @""] autorelease]];
>> +    [menu addItem: [[[NSMenuItem alloc] initWithTitle: @"Resume" action: 
>> @selector(resumeQemu:) keyEquivalent: @""] autorelease]];
> 
> I suggest you split this patch up. The addition of 'pause'/'resume'
> is straightforward because we already do that in the GTK front end.
> Adding support for 'eject floppy/cdrom' is a separate feature and
> should be in its own patch for review (you should cc the block
> maintainers on that to check that we have the right mechanism for
> "find the cdrom drive").

Ok. 


reply via email to

[Prev in Thread] Current Thread [Next in Thread]