On Wed, 10 Nov 2010 14:33:47 +0100 Markus Armbruster <arm...@redhat.com> wrote:
> Luiz Capitulino <lcapitul...@redhat.com> writes: > > > On Wed, 10 Nov 2010 13:56:39 +0100 > > Markus Armbruster <arm...@redhat.com> wrote: > > > >> Luiz Capitulino <lcapitul...@redhat.com> writes: > >> > >> > On Wed, 10 Nov 2010 10:26:15 +0100 > >> > Markus Armbruster <arm...@redhat.com> 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. > [...] >
