From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=60257 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PGAxz-0003uR-3y for qemu-devel@nongnu.org; Wed, 10 Nov 2010 08:44:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PGAxx-00039y-QO for qemu-devel@nongnu.org; Wed, 10 Nov 2010 08:43:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50149) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PGAxx-00039q-H0 for qemu-devel@nongnu.org; Wed, 10 Nov 2010 08:43:57 -0500 Date: Wed, 10 Nov 2010 11:43:52 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 1/2] qemu-char: Introduce Buffered driver Message-ID: <20101110114352.3f1a28ca@doriath> In-Reply-To: References: <1288037204-27768-1-git-send-email-lcapitulino@redhat.com> <1288037204-27768-2-git-send-email-lcapitulino@redhat.com> <20101110103235.1639f23a@doriath> <20101110111228.3140caa8@doriath> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org On Wed, 10 Nov 2010 14:33:47 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > On Wed, 10 Nov 2010 13:56:39 +0100 > > Markus Armbruster wrote: > > > >> Luiz Capitulino writes: > >> > >> > On Wed, 10 Nov 2010 10:26:15 +0100 > >> > Markus Armbruster wrote: > [...] > >> >> Unlike normal character drivers, this one can't be closed with > >> >> qemu_chr_close(), can it? What happens if someone calls > >> >> qemu_chr_close() on it? > >> > > >> > I guess it will explode, because this driver is not in the chardevs list > >> > and our CharDriverState instance is allocated on the stack. > >> > > >> > Does a function comment solves the problem or do you have something else > >> > in mind? > >> > >> A general OO rule: Having different constructors for different sub-types > >> is okay, but once constructed, you should be able to use the objects > >> without knowing of what sub-type they are. That includes destruction. > > > > We will have to add our MemoryDriver to the chardevs list, this has some > > implications like being visible in qemu_chr_info() and qemu_chr_find(), > > likely to also imply that we should choose a chr->filename. > > Not if we formalize the notion of an "internal use only" character > device. Say, !chr->filename means it's internal, and internal ones > aren't in chardevs. Make qemu_chr_close()'s QTAILQ_REMOVE() conditional > !chr->filename. Yes, it's doable. But this kind of change will make this series intrusive, for example, is it really impossible to create !chr->filename via the normal means (eg. from the user)? What if we break something else with this change? > > Another detail is that we'll have to dynamically alocate our CharDriverState > > instance. Not a problem, but adds a few more lines of code and a > > qemu_free(). None of this is needed today. > > I doubt the alloc/free matters. > > > Really worth it? > > Your call. I don't think so, unless we have a real need for it (and this can be done later anyway). > But if you decide not to, please add a suitable assertion to > qemu_chr_close(), to make it obvious what went wrong when an internal > character device explodes there. > > >> Exceptions prove the rule. Maybe this is one, maybe not. > [...] >