On Mon, Nov 30, 2009 at 06:05:31PM +0100, Stefan Weil wrote: > 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 <w...@mail.berlios.de> > >> --- > >> 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.
"Temporary data" comment is very evil. We not only don't want to document code, we document this fact. > The state machine of the real hardware > is not expected to be used in a reentrant way. The same > applies to the emulated hardware. That code does not appear currently structured as a state machine. > Do you expect reentrant calls of transmit or receive > functions for one device instance? > > Regards, > Stefan Datastructures should make sense. This one does not seem to make sense. What's the benefit here? Why is it good to pass less parameters to functions? -- MST