From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1Nz9mq-0004vE-Aj
	for qemu-devel@nongnu.org; Tue, 06 Apr 2010 10:29:52 -0400
Received: from [140.186.70.92] (port=56817 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Nz9mo-0004uS-U7
	for qemu-devel@nongnu.org; Tue, 06 Apr 2010 10:29:51 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <weil@mail.berlios.de>) id 1Nz9ml-0006CI-R6
	for qemu-devel@nongnu.org; Tue, 06 Apr 2010 10:29:50 -0400
Received: from moutng.kundenserver.de ([212.227.126.186]:60884)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <weil@mail.berlios.de>) id 1Nz9ml-0006CB-FB
	for qemu-devel@nongnu.org; Tue, 06 Apr 2010 10:29:47 -0400
Message-ID: <4BBB4559.6030702@mail.berlios.de>
Date: Tue, 06 Apr 2010 16:29:45 +0200
From: Stefan Weil <weil@mail.berlios.de>
MIME-Version: 1.0
References: <1270554249-24861-1-git-send-email-weil@mail.berlios.de>
	<1270554249-24861-3-git-send-email-weil@mail.berlios.de>
	<20100406121830.GE16539@redhat.com>
In-Reply-To: <20100406121830.GE16539@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 2/9] eepro100: Simplify status handling
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>

Michael S. Tsirkin schrieb:
> On Tue, Apr 06, 2010 at 01:44:02PM +0200, Stefan Weil wrote:
>   
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> ---
>>  hw/eepro100.c |    9 +++++----
>>  1 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index 0415132..741031c 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -175,6 +175,7 @@ typedef enum {
>>  } scb_command_bit;
>>  
>>  typedef enum {
>> +    STATUS_NOT_OK = 0,
>>      STATUS_C = BIT(15),
>>      STATUS_OK = BIT(13),
>>  } scb_status_bit;
>>     
>
> 0 is not a bit, I would just use 0 as a constant below.
>   

In a philosophical way, not a bit is some kind of a bit.

I think ok_status = STATUS_NOT_OK is clearer than
ok_status = 0.

>   
>> @@ -882,7 +883,7 @@ static void action_command(EEPRO100State *s)
>>          bool bit_s;
>>          bool bit_i;
>>          bool bit_nc;
>> -        bool success = true;
>> +        uint16_t ok_status = STATUS_OK;
>>          s->cb_address = s->cu_base + s->cu_offset;
>>          read_cb(s);
>>          bit_el = ((s->tx.command & COMMAND_EL) != 0);
>> @@ -915,7 +916,7 @@ static void action_command(EEPRO100State *s)
>>          case CmdTx:
>>              if (bit_nc) {
>>                  missing("CmdTx: NC = 0");
>> -                success = false;
>> +                ok_status = STATUS_NOT_OK;
>>                  break;
>>              }
>>              tx_command(s);
>> @@ -932,11 +933,11 @@ static void action_command(EEPRO100State *s)
>>              break;
>>          default:
>>              missing("undefined command");
>> -            success = false;
>> +            ok_status = STATUS_NOT_OK;
>>              break;
>>          }
>>          /* Write new status. */
>> -        stw_phys(s->cb_address, s->tx.status | STATUS_C | (success ? STATUS_OK : 0));
>> +        stw_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
>>          if (bit_i) {
>>              /* CU completed action. */
>>              eepro100_cx_interrupt(s);
>> -- 
>> 1.7.0
>>