[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v4 5/5] atapi: GESN: implement 'media' subcomman
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v4 5/5] atapi: GESN: implement 'media' subcommand |
Date: |
Tue, 12 Apr 2011 17:13:34 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10 |
Am 12.04.2011 17:03, schrieb Jes Sorensen:
> On 04/12/11 16:09, Amit Shah wrote:
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index dafc049..209d8e6 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -1084,6 +1084,49 @@ static int ide_dvd_read_structure(IDEState *s, int
>> format,
>> }
>> }
>>
>> +static unsigned int event_status_media(IDEState *s,
>> + uint8_t *buf,
>> + unsigned int max_len)
>> +{
>> + enum media_event_code {
>> + no_change = 0, /* Status unchanged */
>> + eject_requested, /* received a request from user to eject */
>> + new_media, /* new media inserted and ready for access */
>> + media_removal, /* only for media changers */
>> + media_changed, /* only for media changers */
>> + bg_format_completed, /* MRW or DVD+RW b/g format completed */
>> + bg_format_restarted, /* MRW or DVD+RW b/g format restarted */
>> + };
>> + enum media_status {
>> + tray_open = 1,
>> + media_present = 2,
>> + };
>
> Would be nice if you used upper-case with the enums since they are
> effectively just defines, to avoid confusing them with variables.
>
>> + uint8_t event_code, media_status;
>> +
>> + media_status = 0;
>> + if (s->bs->tray_open) {
>> + media_status = tray_open;
>> + } else if (bdrv_is_inserted(s->bs)) {
>> + media_status = media_present;
>> + }
>> +
>> + /* Event notification descriptor */
>> + event_code = no_change;
>> + if (media_status != tray_open && s->events.new_media) {
>> + event_code = new_media;
>> + s->events.new_media = false;
>> + }
>> +
>> + buf[4] = event_code;
>> + buf[5] = media_status;
>> +
>> + /* These fields are reserved, just clear them. */
>> + buf[6] = 0;
>> + buf[7] = 0;
>> +
>> + return 8; /* We wrote to 4 extra bytes from the header */
>
> Shouldn't you verify that you don't exceed max_len in this?
Not necessary (the buffer is always 2048 bytes), but it looks like the
max_len parameter is unused now, so it could be removed.
Kevin