From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Lu8Wl-0005au-E1 for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:03:59 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Lu8Wg-0005ZU-D6 for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:03:58 -0400 Received: from [199.232.76.173] (port=44901 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Lu8Wf-0005ZA-Sd for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:03:53 -0400 Received: from lizzard.sbs.de ([194.138.37.39]:19346) by monty-python.gnu.org with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1Lu8We-00036z-Vm for qemu-devel@nongnu.org; Wed, 15 Apr 2009 13:03:53 -0400 Message-ID: <49E61375.6040200@siemens.com> Date: Wed, 15 Apr 2009 19:03:49 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <20090414172954.15035.73053.stgit@mchn012c.ww002.siemens.net> <20090414172954.15035.35711.stgit@mchn012c.ww002.siemens.net> <1239800979.4431.128.camel@blaa> <49E5E953.8060207@siemens.com> <1239814851.4431.147.camel@blaa> In-Reply-To: <1239814851.4431.147.camel@blaa> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH 2/7] net: Add VLAN client cleanup handler Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark McLoughlin Cc: qemu-devel@nongnu.org Mark McLoughlin wrote: > On Wed, 2009-04-15 at 16:04 +0200, Jan Kiszka wrote: >> Mark McLoughlin wrote: >> >>> - Rather than adding yet another param to new_vlan_client(), I just >>> initialize vc->cleanup after creating the client; another patch in >>> my queue removes all callbacks to new_vlan_client() because as more >>> are added it just gets terribly unwieldy. >> Personally, I prefer a function-based API over, well, hacking the >> structures directly. The driver should not have to poke into its device >> descriptor. > > I see this as similar to filing out a vtable. Perhaps it makes sense to > split the function pointers out into a separate struct. > >> Yes, this breaks existing code each time you add another >> callback, but this has the effect that you >> a) normally think twice if you really need it before doing this, >> b) likely enhance all users, and >> c) that new users will find prominent information about this >> (hopefully) useful callback. > > The way I see it is that often when you're adding a new callback, you > don't want to require all users to implement it. Instead you make sure > there is a reasonable default behaviour for when the callback is NULL, > and only set it to non-NULL for the users that really need it. Based on our patches, one could easily argue that the cleanup handler does not fall into this category - nearly everyone wants it, no? Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux