From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NJTmR-0006Pc-VX for qemu-devel@nongnu.org; Sat, 12 Dec 2009 10:21:12 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NJTmM-0006Jc-Eg for qemu-devel@nongnu.org; Sat, 12 Dec 2009 10:21:10 -0500 Received: from [199.232.76.173] (port=52317 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NJTmL-0006JD-LJ for qemu-devel@nongnu.org; Sat, 12 Dec 2009 10:21:06 -0500 Received: from mail-yx0-f188.google.com ([209.85.210.188]:38773) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NJTmL-0004MM-EQ for qemu-devel@nongnu.org; Sat, 12 Dec 2009 10:21:05 -0500 Received: by mail-yx0-f188.google.com with SMTP id 26so1725364yxe.4 for ; Sat, 12 Dec 2009 07:21:05 -0800 (PST) Message-ID: <4B23B4DE.2060604@codemonkey.ws> Date: Sat, 12 Dec 2009 09:21:02 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] eepro100: Restructure code (new function tx_command) References: <1258664086-20264-1-git-send-email-weil@mail.berlios.de> <4B23A2A6.5020201@mail.berlios.de> In-Reply-To: <4B23A2A6.5020201@mail.berlios.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: QEMU Developers , "Michael S. Tsirkin" Stefan Weil wrote: > Stefan Weil schrieb: > >> 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 >> --- >> hw/eepro100.c | 192 >> +++++++++++++++++++++++++++++---------------------------- >> 1 files changed, 98 insertions(+), 94 deletions(-) >> > > This patch is still missing in QEMU master. > Because of other patches (which were sent later!), a rebase is needed again > (it was rebased 3 times now). Only the line with qemu_send_packet changed. > > Please apply the rebased patch if there are no really important > reasons against it (there are none), hopefully without waiting several > weeks again. > It is needed for very important bug fixes (multicast, simple transmit mode). > Michael had given the patch some feedback that I think is valuable. I haven't seen a response to his feedback nor have I seen a resubmit incorporating the feedback. Storing intermediate state of a function as global state (instead of on the stack) is odd. Even if there's a great justification (I don't think there is), such a thing is sufficiently unusual that it deserves a better comment. BTW, part of the trouble your having is you're sending one patch that does too many things. If you submitted a series that contained code cleanups, and then code motion, the first patches would have been no brainers to commit while the code motion would have been treated separately making rebasing less painful. Regards, Anthony Liguori