[Qemu-devel] Re: [PATCH] eepro100: Restructure code (new function tx_command)

[Stefan Weil]

Michael S. Tsirkin schrieb:
> On Thu, Nov 19, 2009 at 09:54:46PM +0100, Stefan Weil wrote:
>> Handling of transmit commands is rather complex,
>> so about 80 lines of code were moved from function
>> action_command to the new function tx_command.
>>
>> The two new values "tx" and "cb_address" in the
>> eepro100 status structure made this possible without
>> passing too many parameters.
>>
>> In addition, the moved code was cleaned a little bit:
>> old comments marked with //~ were removed, C++ style
>> comments were replaced by C style comments, C++ like
>> variable declarations after code were reordered.
>>
>> Simplified mode is still broken. Nor did I fix
>> endianess issues. Both problems will be fixed in
>> additional patches (which need this one).
>>
>> Signed-off-by: Stefan Weil <address@hidden>
>> ---
>> hw/eepro100.c | 192
>> +++++++++++++++++++++++++++++----------------------------
>> 1 files changed, 98 insertions(+), 94 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index 4210d8a..7093af8 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -213,6 +213,10 @@ typedef struct {
>> uint32_t ru_offset; /* RU address offset */
>> uint32_t statsaddr; /* pointer to eepro100_stats_t */
>>
>> + /* Temporary data. */
>> + eepro100_tx_t tx;
>> + uint32_t cb_address;
>> +
>
> That's pretty evil, as it makes routines non-reentrant.
> How bad is it to pass 2 additional arguments around?
> If not, maybe define struct tx_command and put necessary stuff there,
> then pass pointer to that?

Yes, I know that it makes routines non-reentrant, or
to be more exact: it makes routines non-reentrant
for the same device instance. Different device instances
are reentrant because each instance maintains its
own status.

No, it's not evil. The state machine of the real hardware
is not expected to be used in a reentrant way. The same
applies to the emulated hardware.

Do you expect reentrant calls of transmit or receive
functions for one device instance?

Regards,
Stefan